From be07a889816b72c6e73b7649afe563590156ff9c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 12:56:14 +0200 Subject: [PATCH 01/23] block: Use blk_co_flush() for all BB level flushes All read/write functions already have a single coroutine-based function on the BlockBackend level through which all requests go (no matter what API style the external caller used) and which passes the requests down to the block node level. This patch extends this mode of operation to flushes. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/block-backend.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 1a724a8d89..96bb6340a0 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1099,14 +1099,19 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset, blk_aio_write_entry, flags, cb, opaque); } +static void blk_aio_flush_entry(void *opaque) +{ + BlkAioEmAIOCB *acb = opaque; + BlkRwCo *rwco = &acb->rwco; + + rwco->ret = blk_co_flush(rwco->blk); + blk_aio_complete(acb); +} + BlockAIOCB *blk_aio_flush(BlockBackend *blk, BlockCompletionFunc *cb, void *opaque) { - if (!blk_is_available(blk)) { - return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM); - } - - return bdrv_aio_flush(blk_bs(blk), cb, opaque); + return blk_aio_prwv(blk, 0, 0, NULL, blk_aio_flush_entry, 0, cb, opaque); } BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, @@ -1169,13 +1174,15 @@ int blk_co_flush(BlockBackend *blk) return bdrv_co_flush(blk_bs(blk)); } +static void blk_flush_entry(void *opaque) +{ + BlkRwCo *rwco = opaque; + rwco->ret = blk_co_flush(rwco->blk); +} + int blk_flush(BlockBackend *blk) { - if (!blk_is_available(blk)) { - return -ENOMEDIUM; - } - - return bdrv_flush(blk_bs(blk)); + return blk_prw(blk, 0, NULL, 0, blk_flush_entry, 0); } void blk_drain(BlockBackend *blk) From 8c2e3dd55f54ba00fd0893a535c09b5fcd7a877b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 12:56:14 +0200 Subject: [PATCH 02/23] block: Use blk_co_pdiscard() for all BB level discard All read/write functions already have a single coroutine-based function on the BlockBackend level through which all requests go (no matter what API style the external caller used) and which passes the requests down to the block node level. This patch extends this mode of operation to discards. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/block-backend.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 96bb6340a0..39336de330 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1114,16 +1114,21 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk, return blk_aio_prwv(blk, 0, 0, NULL, blk_aio_flush_entry, 0, cb, opaque); } +static void blk_aio_pdiscard_entry(void *opaque) +{ + BlkAioEmAIOCB *acb = opaque; + BlkRwCo *rwco = &acb->rwco; + + rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, acb->bytes); + blk_aio_complete(acb); +} + BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int count, BlockCompletionFunc *cb, void *opaque) { - int ret = blk_check_byte_request(blk, offset, count); - if (ret < 0) { - return blk_abort_aio_request(blk, cb, opaque, ret); - } - - return bdrv_aio_pdiscard(blk_bs(blk), offset, count, cb, opaque); + return blk_aio_prwv(blk, offset, count, NULL, blk_aio_pdiscard_entry, 0, + cb, opaque); } void blk_aio_cancel(BlockAIOCB *acb) @@ -1562,14 +1567,15 @@ int blk_truncate(BlockBackend *blk, int64_t offset) return bdrv_truncate(blk_bs(blk), offset); } +static void blk_pdiscard_entry(void *opaque) +{ + BlkRwCo *rwco = opaque; + rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size); +} + int blk_pdiscard(BlockBackend *blk, int64_t offset, int count) { - int ret = blk_check_byte_request(blk, offset, count); - if (ret < 0) { - return ret; - } - - return bdrv_pdiscard(blk_bs(blk), offset, count); + return blk_prw(blk, offset, NULL, count, blk_pdiscard_entry, 0); } int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf, From 7381e95cc2c33b589c94a857dff21bf2016a08b7 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:40:22 +0200 Subject: [PATCH 03/23] block: Remove bdrv_aio_pdiscard() It is unused now. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/io.c | 29 ----------------------------- block/trace-events | 1 - include/block/block.h | 3 --- 3 files changed, 33 deletions(-) diff --git a/block/io.c b/block/io.c index b136c89ae0..ff93ba1426 100644 --- a/block/io.c +++ b/block/io.c @@ -2196,35 +2196,6 @@ BlockAIOCB *bdrv_aio_flush(BlockDriverState *bs, return &acb->common; } -static void coroutine_fn bdrv_aio_pdiscard_co_entry(void *opaque) -{ - BlockAIOCBCoroutine *acb = opaque; - BlockDriverState *bs = acb->common.bs; - - acb->req.error = bdrv_co_pdiscard(bs, acb->req.offset, acb->req.bytes); - bdrv_co_complete(acb); -} - -BlockAIOCB *bdrv_aio_pdiscard(BlockDriverState *bs, int64_t offset, int count, - BlockCompletionFunc *cb, void *opaque) -{ - Coroutine *co; - BlockAIOCBCoroutine *acb; - - trace_bdrv_aio_pdiscard(bs, offset, count, opaque); - - acb = qemu_aio_get(&bdrv_em_co_aiocb_info, bs, cb, opaque); - acb->need_bh = true; - acb->req.error = -EINPROGRESS; - acb->req.offset = offset; - acb->req.bytes = count; - co = qemu_coroutine_create(bdrv_aio_pdiscard_co_entry, acb); - qemu_coroutine_enter(co); - - bdrv_co_maybe_schedule_bh(acb); - return &acb->common; -} - void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs, BlockCompletionFunc *cb, void *opaque) { diff --git a/block/trace-events b/block/trace-events index 05fa13c891..aff8a9674d 100644 --- a/block/trace-events +++ b/block/trace-events @@ -9,7 +9,6 @@ blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags blk_co_pwritev(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags %x" # block/io.c -bdrv_aio_pdiscard(void *bs, int64_t offset, int count, void *opaque) "bs %p offset %"PRId64" count %d opaque %p" bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p" bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" diff --git a/include/block/block.h b/include/block/block.h index 107c603605..99a15a6bf6 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -314,9 +314,6 @@ BlockAIOCB *bdrv_aio_writev(BdrvChild *child, int64_t sector_num, BlockCompletionFunc *cb, void *opaque); BlockAIOCB *bdrv_aio_flush(BlockDriverState *bs, BlockCompletionFunc *cb, void *opaque); -BlockAIOCB *bdrv_aio_pdiscard(BlockDriverState *bs, - int64_t offset, int count, - BlockCompletionFunc *cb, void *opaque); void bdrv_aio_cancel(BlockAIOCB *acb); void bdrv_aio_cancel_async(BlockAIOCB *acb); From 48af776a5b85ad2dc6124d3d0fb210ca6b98d32c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 12:56:14 +0200 Subject: [PATCH 04/23] block: Use blk_co_ioctl() for all BB level ioctls All read/write functions already have a single coroutine-based function on the BlockBackend level through which all requests go (no matter what API style the external caller used) and which passes the requests down to the block node level. This patch exports a bdrv_co_ioctl() function and uses it to extend this mode of operation to ioctls. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/block-backend.c | 39 ++++++++++++++++++++++++++++------ block/io.c | 8 +++---- include/block/block.h | 1 + include/sysemu/block-backend.h | 1 + 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 39336de330..c53ca30000 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1141,23 +1141,50 @@ void blk_aio_cancel_async(BlockAIOCB *acb) bdrv_aio_cancel_async(acb); } -int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf) +int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf) { if (!blk_is_available(blk)) { return -ENOMEDIUM; } - return bdrv_ioctl(blk_bs(blk), req, buf); + return bdrv_co_ioctl(blk_bs(blk), req, buf); +} + +static void blk_ioctl_entry(void *opaque) +{ + BlkRwCo *rwco = opaque; + rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset, + rwco->qiov->iov[0].iov_base); +} + +int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf) +{ + return blk_prw(blk, req, buf, 0, blk_ioctl_entry, 0); +} + +static void blk_aio_ioctl_entry(void *opaque) +{ + BlkAioEmAIOCB *acb = opaque; + BlkRwCo *rwco = &acb->rwco; + + rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset, + rwco->qiov->iov[0].iov_base); + blk_aio_complete(acb); } BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf, BlockCompletionFunc *cb, void *opaque) { - if (!blk_is_available(blk)) { - return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM); - } + QEMUIOVector qiov; + struct iovec iov; - return bdrv_aio_ioctl(blk_bs(blk), req, buf, cb, opaque); + iov = (struct iovec) { + .iov_base = buf, + .iov_len = 0, + }; + qemu_iovec_init_external(&qiov, &iov, 1); + + return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb, opaque); } int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count) diff --git a/block/io.c b/block/io.c index ff93ba1426..7c119d5113 100644 --- a/block/io.c +++ b/block/io.c @@ -2492,7 +2492,7 @@ int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, int count) return rwco.ret; } -static int bdrv_co_do_ioctl(BlockDriverState *bs, int req, void *buf) +int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf) { BlockDriver *drv = bs->drv; BdrvTrackedRequest tracked_req; @@ -2528,7 +2528,7 @@ typedef struct { static void coroutine_fn bdrv_co_ioctl_entry(void *opaque) { BdrvIoctlCoData *data = opaque; - data->ret = bdrv_co_do_ioctl(data->bs, data->req, data->buf); + data->ret = bdrv_co_ioctl(data->bs, data->req, data->buf); } /* needed for generic scsi interface */ @@ -2558,8 +2558,8 @@ int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) static void coroutine_fn bdrv_co_aio_ioctl_entry(void *opaque) { BlockAIOCBCoroutine *acb = opaque; - acb->req.error = bdrv_co_do_ioctl(acb->common.bs, - acb->req.req, acb->req.buf); + acb->req.error = bdrv_co_ioctl(acb->common.bs, + acb->req.req, acb->req.buf); bdrv_co_complete(acb); } diff --git a/include/block/block.h b/include/block/block.h index 99a15a6bf6..e06db62ad3 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -318,6 +318,7 @@ void bdrv_aio_cancel(BlockAIOCB *acb); void bdrv_aio_cancel_async(BlockAIOCB *acb); /* sg packet commands */ +int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf); int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf); BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, unsigned long int req, void *buf, diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index b07159b639..6444e41d39 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -146,6 +146,7 @@ BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int count, BlockCompletionFunc *cb, void *opaque); void blk_aio_cancel(BlockAIOCB *acb); void blk_aio_cancel_async(BlockAIOCB *acb); +int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf); int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf); BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf, BlockCompletionFunc *cb, void *opaque); From 0d4377b3ea97cc28c71642c92c2cfa8cdb1cc8bd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:50:12 +0200 Subject: [PATCH 05/23] raw-posix: Don't use bdrv_ioctl() Instead of letting raw-posix use the bdrv_ioctl() abstraction to issue an ioctl to itself, just call ioctl() directly. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/raw-posix.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index f481e57f78..247e47b88f 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -2069,13 +2069,23 @@ static bool hdev_is_sg(BlockDriverState *bs) #if defined(__linux__) + BDRVRawState *s = bs->opaque; struct stat st; struct sg_scsi_id scsiid; int sg_version; + int ret; - if (stat(bs->filename, &st) >= 0 && S_ISCHR(st.st_mode) && - !bdrv_ioctl(bs, SG_GET_VERSION_NUM, &sg_version) && - !bdrv_ioctl(bs, SG_GET_SCSI_ID, &scsiid)) { + if (stat(bs->filename, &st) < 0 || !S_ISCHR(st.st_mode)) { + return false; + } + + ret = ioctl(s->fd, SG_GET_VERSION_NUM, &sg_version); + if (ret < 0) { + return false; + } + + ret = ioctl(s->fd, SG_GET_SCSI_ID, &scsiid); + if (ret >= 0) { DPRINTF("SG device found: type=%d, version=%d\n", scsiid.scsi_type, sg_version); return true; From 61b2450414ee052c8f2561e999852fad0534899e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:40:22 +0200 Subject: [PATCH 06/23] block: Remove bdrv_ioctl() It is unused now. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/io.c | 37 ------------------------------------- include/block/block.h | 1 - 2 files changed, 38 deletions(-) diff --git a/block/io.c b/block/io.c index 7c119d5113..35fdcca747 100644 --- a/block/io.c +++ b/block/io.c @@ -2518,43 +2518,6 @@ out: return co.ret; } -typedef struct { - BlockDriverState *bs; - int req; - void *buf; - int ret; -} BdrvIoctlCoData; - -static void coroutine_fn bdrv_co_ioctl_entry(void *opaque) -{ - BdrvIoctlCoData *data = opaque; - data->ret = bdrv_co_ioctl(data->bs, data->req, data->buf); -} - -/* needed for generic scsi interface */ -int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) -{ - BdrvIoctlCoData data = { - .bs = bs, - .req = req, - .buf = buf, - .ret = -EINPROGRESS, - }; - - if (qemu_in_coroutine()) { - /* Fast-path if already in coroutine context */ - bdrv_co_ioctl_entry(&data); - } else { - Coroutine *co = qemu_coroutine_create(bdrv_co_ioctl_entry, &data); - - qemu_coroutine_enter(co); - while (data.ret == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(bs), true); - } - } - return data.ret; -} - static void coroutine_fn bdrv_co_aio_ioctl_entry(void *opaque) { BlockAIOCBCoroutine *acb = opaque; diff --git a/include/block/block.h b/include/block/block.h index e06db62ad3..e0a54aa816 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -319,7 +319,6 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb); /* sg packet commands */ int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf); -int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf); BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, unsigned long int req, void *buf, BlockCompletionFunc *cb, void *opaque); From 16a389dc9ecc05e9d8d6ebd3eff6dc98158523e0 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 15:07:27 +0200 Subject: [PATCH 07/23] block: Introduce .bdrv_co_ioctl() driver callback This allows drivers to implement ioctls in a coroutine-based way. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/io.c | 16 ++++++++++------ include/block/block_int.h | 2 ++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/block/io.c b/block/io.c index 35fdcca747..370c7d8128 100644 --- a/block/io.c +++ b/block/io.c @@ -2502,17 +2502,21 @@ int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf) BlockAIOCB *acb; tracked_request_begin(&tracked_req, bs, 0, 0, BDRV_TRACKED_IOCTL); - if (!drv || !drv->bdrv_aio_ioctl) { + if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) { co.ret = -ENOTSUP; goto out; } - acb = drv->bdrv_aio_ioctl(bs, req, buf, bdrv_co_io_em_complete, &co); - if (!acb) { - co.ret = -ENOTSUP; - goto out; + if (drv->bdrv_co_ioctl) { + co.ret = drv->bdrv_co_ioctl(bs, req, buf); + } else { + acb = drv->bdrv_aio_ioctl(bs, req, buf, bdrv_co_io_em_complete, &co); + if (!acb) { + co.ret = -ENOTSUP; + goto out; + } + qemu_coroutine_yield(); } - qemu_coroutine_yield(); out: tracked_request_end(&tracked_req); return co.ret; diff --git a/include/block/block_int.h b/include/block/block_int.h index 3e79228eb0..e96e9ada57 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -244,6 +244,8 @@ struct BlockDriver { BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf, BlockCompletionFunc *cb, void *opaque); + int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs, + unsigned long int req, void *buf); /* List of options for creating images, terminated by name == NULL */ QemuOptsList *create_opts; From 151a2930c437e9f09f6b1c21c99ccabfd5dba9c7 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 15:09:36 +0200 Subject: [PATCH 08/23] raw: Implement .bdrv_co_ioctl instead of .bdrv_aio_ioctl It's the simpler interface to use for the raw format driver. Apart from that, this removes the last user of the AIO emulation implemented by bdrv_aio_ioctl(). Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/raw_bsd.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 588d4080f9..fc16ec1f74 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -176,12 +176,9 @@ static void raw_lock_medium(BlockDriverState *bs, bool locked) bdrv_lock_medium(bs->file->bs, locked); } -static BlockAIOCB *raw_aio_ioctl(BlockDriverState *bs, - unsigned long int req, void *buf, - BlockCompletionFunc *cb, - void *opaque) +static int raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) { - return bdrv_aio_ioctl(bs->file->bs, req, buf, cb, opaque); + return bdrv_co_ioctl(bs->file->bs, req, buf); } static int raw_has_zero_init(BlockDriverState *bs) @@ -261,7 +258,7 @@ BlockDriver bdrv_raw = { .bdrv_media_changed = &raw_media_changed, .bdrv_eject = &raw_eject, .bdrv_lock_medium = &raw_lock_medium, - .bdrv_aio_ioctl = &raw_aio_ioctl, + .bdrv_co_ioctl = &raw_co_ioctl, .create_opts = &raw_create_opts, .bdrv_has_zero_init = &raw_has_zero_init }; From cbc14ac9c3e199df759e2847b2301e2b990a8537 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:40:22 +0200 Subject: [PATCH 09/23] block: Remove bdrv_aio_ioctl() It is unused now. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/io.c | 27 --------------------------- include/block/block.h | 3 --- 2 files changed, 30 deletions(-) diff --git a/block/io.c b/block/io.c index 370c7d8128..79cbbdf769 100644 --- a/block/io.c +++ b/block/io.c @@ -2522,33 +2522,6 @@ out: return co.ret; } -static void coroutine_fn bdrv_co_aio_ioctl_entry(void *opaque) -{ - BlockAIOCBCoroutine *acb = opaque; - acb->req.error = bdrv_co_ioctl(acb->common.bs, - acb->req.req, acb->req.buf); - bdrv_co_complete(acb); -} - -BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, - unsigned long int req, void *buf, - BlockCompletionFunc *cb, void *opaque) -{ - BlockAIOCBCoroutine *acb = qemu_aio_get(&bdrv_em_co_aiocb_info, - bs, cb, opaque); - Coroutine *co; - - acb->need_bh = true; - acb->req.error = -EINPROGRESS; - acb->req.req = req; - acb->req.buf = buf; - co = qemu_coroutine_create(bdrv_co_aio_ioctl_entry, acb); - qemu_coroutine_enter(co); - - bdrv_co_maybe_schedule_bh(acb); - return &acb->common; -} - void *qemu_blockalign(BlockDriverState *bs, size_t size) { return qemu_memalign(bdrv_opt_mem_align(bs), size); diff --git a/include/block/block.h b/include/block/block.h index e0a54aa816..398a050176 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -319,9 +319,6 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb); /* sg packet commands */ int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf); -BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, - unsigned long int req, void *buf, - BlockCompletionFunc *cb, void *opaque); /* Invalidate any cached metadata used by image formats */ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp); From e5b77eec91eb03101d53d8ce4a5d03e40b5d1000 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 27 Oct 2016 13:16:56 +0200 Subject: [PATCH 10/23] qemu-iotests: Fix typo for NFS with IMGOPTSSYNTAX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 076003f5 added configuration for NFS with IMGOPTSSYNTAX enabled, but it didn't use the right variable name: $TEST_DIR_OPTS doesn't exist. This fixes the mistake. However, this doesn't make anything work that was broken before: The only way to get IMGOPTSSYNTAX is with -luks, but the combination of -luks and -nfs doesn't get qemu-img create commands right (because qemu-img create doesn't support --image-opts yet), so even after this fix some more work would be required to make the tests pass. Reported-by: Tomáš Golembiovský Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 126bd67043..3213765f4e 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -69,7 +69,7 @@ if [ "$IMGOPTSSYNTAX" = "true" ]; then TEST_IMG="$DRIVER,file.driver=ssh,file.host=127.0.0.1,file.path=$TEST_IMG_FILE" elif [ "$IMGPROTO" = "nfs" ]; then TEST_DIR="$DRIVER,file.driver=nfs,file.filename=nfs://127.0.0.1/$TEST_DIR" - TEST_IMG=$TEST_DIR_OPTS/t.$IMGFMT + TEST_IMG=$TEST_DIR/t.$IMGFMT elif [ "$IMGPROTO" = "archipelago" ]; then TEST_IMG="$DRIVER,file.driver=archipelago,file.volume=:at.$IMGFMT" else From 82d73014a9111c53fefb7e7954c9e370d23f7569 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:29 +0200 Subject: [PATCH 11/23] block/nbd: Drop trailing "." in error messages Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 4 ++-- tests/qemu-iotests/051.out | 4 ++-- tests/qemu-iotests/051.pc.out | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index 6bc06d6198..ce7c14f8df 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -200,9 +200,9 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp) if (!s->path == !s->host) { if (s->path) { - error_setg(errp, "path and host may not be used at the same time."); + error_setg(errp, "path and host may not be used at the same time"); } else { - error_setg(errp, "one of path and host must be specified."); + error_setg(errp, "one of path and host must be specified"); } return NULL; } diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 408d613bc1..9e584fe7b6 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -222,7 +222,7 @@ Testing: -drive driver=file QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name Testing: -drive driver=nbd -QEMU_PROG: -drive driver=nbd: one of path and host must be specified. +QEMU_PROG: -drive driver=nbd: one of path and host must be specified Testing: -drive driver=raw QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the protocol level @@ -231,7 +231,7 @@ Testing: -drive file.driver=file QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file name Testing: -drive file.driver=nbd -QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified. +QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified Testing: -drive file.driver=raw QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the protocol level diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index ec6d22229c..6395a30a16 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -316,7 +316,7 @@ Testing: -drive driver=file QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name Testing: -drive driver=nbd -QEMU_PROG: -drive driver=nbd: one of path and host must be specified. +QEMU_PROG: -drive driver=nbd: one of path and host must be specified Testing: -drive driver=raw QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the protocol level @@ -325,7 +325,7 @@ Testing: -drive file.driver=file QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file name Testing: -drive file.driver=nbd -QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified. +QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified Testing: -drive file.driver=raw QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the protocol level From 442045cbce010688eed2b8ede6141e982c6d6b23 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:30 +0200 Subject: [PATCH 12/23] block/nbd: Reject port parameter without host Currently, a port that is passed along with a UNIX socket path is silently ignored. That is not exactly ideal, it should be an error instead. Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index ce7c14f8df..eaca33c35c 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -197,6 +197,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp) s->path = g_strdup(qemu_opt_get(opts, "path")); s->host = g_strdup(qemu_opt_get(opts, "host")); + s->port = g_strdup(qemu_opt_get(opts, "port")); if (!s->path == !s->host) { if (s->path) { @@ -206,6 +207,10 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp) } return NULL; } + if (s->port && !s->host) { + error_setg(errp, "port may not be used without host"); + return NULL; + } saddr = g_new0(SocketAddress, 1); @@ -217,8 +222,6 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp) } else { InetSocketAddress *inet; - s->port = g_strdup(qemu_opt_get(opts, "port")); - saddr->type = SOCKET_ADDRESS_KIND_INET; inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1); inet->host = g_strdup(s->host); From 7edca33804f9b6948a65c04f969facf02a59e6fe Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:31 +0200 Subject: [PATCH 13/23] block/nbd: Default port in nbd_refresh_filename() Instead of not emitting the port in nbd_refresh_filename(), just set it to the default if the user did not specify it. This makes the logic a bit simpler. Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index eaca33c35c..c77a9699fe 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -444,6 +444,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options) { BDRVNBDState *s = bs->opaque; QDict *opts = qdict_new(); + const char *port = s->port ?: stringify(NBD_DEFAULT_PORT); qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd"))); @@ -453,27 +454,19 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options) } else if (s->path && !s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), "nbd+unix://?socket=%s", s->path); - } else if (!s->path && s->export && s->port) { + } else if (!s->path && s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s:%s/%s", s->host, s->port, s->export); - } else if (!s->path && s->export && !s->port) { + "nbd://%s:%s/%s", s->host, port, s->export); + } else if (!s->path && !s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s/%s", s->host, s->export); - } else if (!s->path && !s->export && s->port) { - snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s:%s", s->host, s->port); - } else if (!s->path && !s->export && !s->port) { - snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s", s->host); + "nbd://%s:%s", s->host, port); } if (s->path) { qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(s->path))); - } else if (s->port) { - qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host))); - qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(s->port))); } else { qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host))); + qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port))); } if (s->export) { qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(s->export))); From fcfcd8ffccd81b6fc13a730e2a75cefc5d1eb752 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:32 +0200 Subject: [PATCH 14/23] block/nbd: Use qdict_put() Instead of inlining this nice macro (i.e. resorting to qdict_put_obj(..., QOBJECT(...))), use it. Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index c77a9699fe..c539fb5ff2 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -446,7 +446,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options) QDict *opts = qdict_new(); const char *port = s->port ?: stringify(NBD_DEFAULT_PORT); - qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd"))); + qdict_put(opts, "driver", qstring_from_str("nbd")); if (s->path && s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), @@ -463,17 +463,16 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options) } if (s->path) { - qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(s->path))); + qdict_put(opts, "path", qstring_from_str(s->path)); } else { - qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host))); - qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port))); + qdict_put(opts, "host", qstring_from_str(s->host)); + qdict_put(opts, "port", qstring_from_str(port)); } if (s->export) { - qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(s->export))); + qdict_put(opts, "export", qstring_from_str(s->export)); } if (s->tlscredsid) { - qdict_put_obj(opts, "tls-creds", - QOBJECT(qstring_from_str(s->tlscredsid))); + qdict_put(opts, "tls-creds", qstring_from_str(s->tlscredsid)); } bs->full_open_options = opts; From 48c38e0b8d2d71ec82928bd4b4136a941d39de3c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:33 +0200 Subject: [PATCH 15/23] block/nbd: Add nbd_has_filename_options_conflict() Right now, we have four possible options that conflict with specifying an NBD filename, and a future patch will add another one ("address"). This future option is a nested QDict that is flattened at this point, requiring us to test each option whether its key has an "address." prefix. Therefore, we will then need to iterate through all options (including the "export" option which was not covered so far). Adding this iteration logic now will simplify adding the new option later. A nice side effect is that the user will not receive a long list of five options which are not supposed to be specified with a filename, but we can actually print the problematic option. Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index c539fb5ff2..cdab20fb00 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -123,6 +123,25 @@ out: return ret; } +static bool nbd_has_filename_options_conflict(QDict *options, Error **errp) +{ + const QDictEntry *e; + + for (e = qdict_first(options); e; e = qdict_next(options, e)) { + if (!strcmp(e->key, "host") || + !strcmp(e->key, "port") || + !strcmp(e->key, "path") || + !strcmp(e->key, "export")) + { + error_setg(errp, "Option '%s' cannot be used with a file name", + e->key); + return true; + } + } + + return false; +} + static void nbd_parse_filename(const char *filename, QDict *options, Error **errp) { @@ -131,12 +150,7 @@ static void nbd_parse_filename(const char *filename, QDict *options, const char *host_spec; const char *unixpath; - if (qdict_haskey(options, "host") - || qdict_haskey(options, "port") - || qdict_haskey(options, "path")) - { - error_setg(errp, "host/port/path and a file name may not be specified " - "at the same time"); + if (nbd_has_filename_options_conflict(options, errp)) { return; } From 491d6c7c4e4793ae5b009e120139ad62f56b5a35 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:34 +0200 Subject: [PATCH 16/23] block/nbd: Accept SocketAddress Add a new option "server" to the NBD block driver which accepts a SocketAddress. "path", "host" and "port" are still supported as legacy options and are mapped to their corresponding SocketAddress representation. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 183 ++++++++++++++++++++++------------ tests/qemu-iotests/051.out | 4 +- tests/qemu-iotests/051.pc.out | 4 +- 3 files changed, 121 insertions(+), 70 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index cdab20fb00..a778692a13 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -32,6 +32,9 @@ #include "qemu/uri.h" #include "block/block_int.h" #include "qemu/module.h" +#include "qapi-visit.h" +#include "qapi/qobject-input-visitor.h" +#include "qapi/qobject-output-visitor.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qjson.h" #include "qapi/qmp/qint.h" @@ -44,7 +47,8 @@ typedef struct BDRVNBDState { NbdClientSession client; /* For nbd_refresh_filename() */ - char *path, *host, *port, *export, *tlscredsid; + SocketAddress *saddr; + char *export, *tlscredsid; } BDRVNBDState; static int nbd_parse_uri(const char *filename, QDict *options) @@ -131,7 +135,8 @@ static bool nbd_has_filename_options_conflict(QDict *options, Error **errp) if (!strcmp(e->key, "host") || !strcmp(e->key, "port") || !strcmp(e->key, "path") || - !strcmp(e->key, "export")) + !strcmp(e->key, "export") || + strstart(e->key, "server.", NULL)) { error_setg(errp, "Option '%s' cannot be used with a file name", e->key); @@ -205,50 +210,81 @@ out: g_free(file); } -static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp) +static bool nbd_process_legacy_socket_options(QDict *output_options, + QemuOpts *legacy_opts, + Error **errp) { - SocketAddress *saddr; + const char *path = qemu_opt_get(legacy_opts, "path"); + const char *host = qemu_opt_get(legacy_opts, "host"); + const char *port = qemu_opt_get(legacy_opts, "port"); + const QDictEntry *e; - s->path = g_strdup(qemu_opt_get(opts, "path")); - s->host = g_strdup(qemu_opt_get(opts, "host")); - s->port = g_strdup(qemu_opt_get(opts, "port")); - - if (!s->path == !s->host) { - if (s->path) { - error_setg(errp, "path and host may not be used at the same time"); - } else { - error_setg(errp, "one of path and host must be specified"); - } - return NULL; - } - if (s->port && !s->host) { - error_setg(errp, "port may not be used without host"); - return NULL; + if (!path && !host && !port) { + return true; } - saddr = g_new0(SocketAddress, 1); - - if (s->path) { - UnixSocketAddress *q_unix; - saddr->type = SOCKET_ADDRESS_KIND_UNIX; - q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1); - q_unix->path = g_strdup(s->path); - } else { - InetSocketAddress *inet; - - saddr->type = SOCKET_ADDRESS_KIND_INET; - inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1); - inet->host = g_strdup(s->host); - inet->port = g_strdup(s->port); - if (!inet->port) { - inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT); + for (e = qdict_first(output_options); e; e = qdict_next(output_options, e)) + { + if (strstart(e->key, "server.", NULL)) { + error_setg(errp, "Cannot use 'server' and path/host/port at the " + "same time"); + return false; } } + if (path && host) { + error_setg(errp, "path and host may not be used at the same time"); + return false; + } else if (path) { + if (port) { + error_setg(errp, "port may not be used without host"); + return false; + } + + qdict_put(output_options, "server.type", qstring_from_str("unix")); + qdict_put(output_options, "server.data.path", qstring_from_str(path)); + } else if (host) { + qdict_put(output_options, "server.type", qstring_from_str("inet")); + qdict_put(output_options, "server.data.host", qstring_from_str(host)); + qdict_put(output_options, "server.data.port", + qstring_from_str(port ?: stringify(NBD_DEFAULT_PORT))); + } + + return true; +} + +static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp) +{ + SocketAddress *saddr = NULL; + QDict *addr = NULL; + QObject *crumpled_addr = NULL; + Visitor *iv = NULL; + Error *local_err = NULL; + + qdict_extract_subqdict(options, &addr, "server."); + if (!qdict_size(addr)) { + error_setg(errp, "NBD server address missing"); + goto done; + } + + crumpled_addr = qdict_crumple(addr, errp); + if (!crumpled_addr) { + goto done; + } + + iv = qobject_input_visitor_new(crumpled_addr, true); + visit_type_SocketAddress(iv, NULL, &saddr, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto done; + } + s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX; - s->export = g_strdup(qemu_opt_get(opts, "export")); - +done: + QDECREF(addr); + qobject_decref(crumpled_addr); + visit_free(iv); return saddr; } @@ -349,7 +385,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, QemuOpts *opts = NULL; Error *local_err = NULL; QIOChannelSocket *sioc = NULL; - SocketAddress *saddr = NULL; QCryptoTLSCreds *tlscreds = NULL; const char *hostname = NULL; int ret = -EINVAL; @@ -361,12 +396,19 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, goto error; } - /* Pop the config into our state object. Exit if invalid. */ - saddr = nbd_config(s, opts, errp); - if (!saddr) { + /* Translate @host, @port, and @path to a SocketAddress */ + if (!nbd_process_legacy_socket_options(options, opts, errp)) { goto error; } + /* Pop the config into our state object. Exit if invalid. */ + s->saddr = nbd_config(s, options, errp); + if (!s->saddr) { + goto error; + } + + s->export = g_strdup(qemu_opt_get(opts, "export")); + s->tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds")); if (s->tlscredsid) { tlscreds = nbd_get_tls_creds(s->tlscredsid, errp); @@ -374,17 +416,17 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, goto error; } - if (saddr->type != SOCKET_ADDRESS_KIND_INET) { + if (s->saddr->type != SOCKET_ADDRESS_KIND_INET) { error_setg(errp, "TLS only supported over IP sockets"); goto error; } - hostname = saddr->u.inet.data->host; + hostname = s->saddr->u.inet.data->host; } /* establish TCP connection, return error if it fails * TODO: Configurable retry-until-timeout behaviour. */ - sioc = nbd_establish_connection(saddr, errp); + sioc = nbd_establish_connection(s->saddr, errp); if (!sioc) { ret = -ECONNREFUSED; goto error; @@ -401,13 +443,10 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, object_unref(OBJECT(tlscreds)); } if (ret < 0) { - g_free(s->path); - g_free(s->host); - g_free(s->port); + qapi_free_SocketAddress(s->saddr); g_free(s->export); g_free(s->tlscredsid); } - qapi_free_SocketAddress(saddr); qemu_opts_del(opts); return ret; } @@ -429,9 +468,7 @@ static void nbd_close(BlockDriverState *bs) nbd_client_close(bs); - g_free(s->path); - g_free(s->host); - g_free(s->port); + qapi_free_SocketAddress(s->saddr); g_free(s->export); g_free(s->tlscredsid); } @@ -458,30 +495,43 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options) { BDRVNBDState *s = bs->opaque; QDict *opts = qdict_new(); - const char *port = s->port ?: stringify(NBD_DEFAULT_PORT); + QObject *saddr_qdict; + Visitor *ov; + const char *host = NULL, *port = NULL, *path = NULL; + + if (s->saddr->type == SOCKET_ADDRESS_KIND_INET) { + const InetSocketAddress *inet = s->saddr->u.inet.data; + if (!inet->has_ipv4 && !inet->has_ipv6 && !inet->has_to) { + host = inet->host; + port = inet->port; + } + } else if (s->saddr->type == SOCKET_ADDRESS_KIND_UNIX) { + path = s->saddr->u.q_unix.data->path; + } qdict_put(opts, "driver", qstring_from_str("nbd")); - if (s->path && s->export) { + if (path && s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd+unix:///%s?socket=%s", s->export, s->path); - } else if (s->path && !s->export) { + "nbd+unix:///%s?socket=%s", s->export, path); + } else if (path && !s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd+unix://?socket=%s", s->path); - } else if (!s->path && s->export) { + "nbd+unix://?socket=%s", path); + } else if (host && s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s:%s/%s", s->host, port, s->export); - } else if (!s->path && !s->export) { + "nbd://%s:%s/%s", host, port, s->export); + } else if (host && !s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s:%s", s->host, port); + "nbd://%s:%s", host, port); } - if (s->path) { - qdict_put(opts, "path", qstring_from_str(s->path)); - } else { - qdict_put(opts, "host", qstring_from_str(s->host)); - qdict_put(opts, "port", qstring_from_str(port)); - } + ov = qobject_output_visitor_new(&saddr_qdict); + visit_type_SocketAddress(ov, NULL, &s->saddr, &error_abort); + visit_complete(ov, &saddr_qdict); + assert(qobject_type(saddr_qdict) == QTYPE_QDICT); + + qdict_put_obj(opts, "server", saddr_qdict); + if (s->export) { qdict_put(opts, "export", qstring_from_str(s->export)); } @@ -489,6 +539,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options) qdict_put(opts, "tls-creds", qstring_from_str(s->tlscredsid)); } + qdict_flatten(opts); bs->full_open_options = opts; } diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 9e584fe7b6..42bf4164ca 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -222,7 +222,7 @@ Testing: -drive driver=file QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name Testing: -drive driver=nbd -QEMU_PROG: -drive driver=nbd: one of path and host must be specified +QEMU_PROG: -drive driver=nbd: NBD server address missing Testing: -drive driver=raw QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the protocol level @@ -231,7 +231,7 @@ Testing: -drive file.driver=file QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file name Testing: -drive file.driver=nbd -QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified +QEMU_PROG: -drive file.driver=nbd: NBD server address missing Testing: -drive file.driver=raw QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the protocol level diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index 6395a30a16..603bb768d6 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -316,7 +316,7 @@ Testing: -drive driver=file QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name Testing: -drive driver=nbd -QEMU_PROG: -drive driver=nbd: one of path and host must be specified +QEMU_PROG: -drive driver=nbd: NBD server address missing Testing: -drive driver=raw QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the protocol level @@ -325,7 +325,7 @@ Testing: -drive file.driver=file QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file name Testing: -drive file.driver=nbd -QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified +QEMU_PROG: -drive file.driver=nbd: NBD server address missing Testing: -drive file.driver=raw QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the protocol level From f84d431b86f6f00a8fcefb44af08243e70466acb Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:35 +0200 Subject: [PATCH 17/23] block/nbd: Use SocketAddress options Drop the use of legacy options in favor of the SocketAddress representation, even for internal use (i.e. for storing the result of the filename parsing). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index a778692a13..8ef143870f 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -94,9 +94,13 @@ static int nbd_parse_uri(const char *filename, QDict *options) ret = -EINVAL; goto out; } - qdict_put(options, "path", qstring_from_str(qp->p[0].value)); + qdict_put(options, "server.type", qstring_from_str("unix")); + qdict_put(options, "server.data.path", + qstring_from_str(qp->p[0].value)); } else { QString *host; + char *port_str; + /* nbd[+tcp]://host[:port]/export */ if (!uri->server) { ret = -EINVAL; @@ -111,12 +115,12 @@ static int nbd_parse_uri(const char *filename, QDict *options) host = qstring_from_str(uri->server); } - qdict_put(options, "host", host); - if (uri->port) { - char* port_str = g_strdup_printf("%d", uri->port); - qdict_put(options, "port", qstring_from_str(port_str)); - g_free(port_str); - } + qdict_put(options, "server.type", qstring_from_str("inet")); + qdict_put(options, "server.data.host", host); + + port_str = g_strdup_printf("%d", uri->port ?: NBD_DEFAULT_PORT); + qdict_put(options, "server.data.port", qstring_from_str(port_str)); + g_free(port_str); } out: @@ -192,7 +196,8 @@ static void nbd_parse_filename(const char *filename, QDict *options, /* are we a UNIX or TCP socket? */ if (strstart(host_spec, "unix:", &unixpath)) { - qdict_put(options, "path", qstring_from_str(unixpath)); + qdict_put(options, "server.type", qstring_from_str("unix")); + qdict_put(options, "server.data.path", qstring_from_str(unixpath)); } else { InetSocketAddress *addr = NULL; @@ -201,8 +206,9 @@ static void nbd_parse_filename(const char *filename, QDict *options, goto out; } - qdict_put(options, "host", qstring_from_str(addr->host)); - qdict_put(options, "port", qstring_from_str(addr->port)); + qdict_put(options, "server.type", qstring_from_str("inet")); + qdict_put(options, "server.data.host", qstring_from_str(addr->host)); + qdict_put(options, "server.data.port", qstring_from_str(addr->port)); qapi_free_InetSocketAddress(addr); } From 6b02b1f0a4a5d887e6e773e9f13fffcb2adfdf0d Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:36 +0200 Subject: [PATCH 18/23] qapi: Allow blockdev-add for NBD Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- qapi/block-core.json | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 97b120532a..cd1fa7ba07 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1703,15 +1703,16 @@ # # @host_device, @host_cdrom: Since 2.1 # @gluster: Since 2.7 +# @nbd: Since 2.8 # # Since: 2.0 ## { 'enum': 'BlockdevDriver', 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop', 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', - 'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co', - 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', - 'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] } + 'host_device', 'http', 'https', 'luks', 'nbd', 'null-aio', + 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', + 'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] } ## # @BlockdevOptionsFile @@ -2219,6 +2220,24 @@ { 'struct': 'BlockdevOptionsCurl', 'data': { 'filename': 'str' } } +## +# @BlockdevOptionsNbd +# +# Driver specific block device options for NBD. +# +# @server: NBD server address +# +# @export: #optional export name +# +# @tls-creds: #optional TLS credentials ID +# +# Since: 2.8 +## +{ 'struct': 'BlockdevOptionsNbd', + 'data': { 'server': 'SocketAddress', + '*export': 'str', + '*tls-creds': 'str' } } + ## # @BlockdevOptions # @@ -2264,7 +2283,7 @@ 'https': 'BlockdevOptionsCurl', # TODO iscsi: Wait for structured options 'luks': 'BlockdevOptionsLUKS', -# TODO nbd: Should take InetSocketAddress for 'host'? + 'nbd': 'BlockdevOptionsNbd', # TODO nfs: Wait for structured options 'null-aio': 'BlockdevOptionsNull', 'null-co': 'BlockdevOptionsNull', From bec87774c24aac5b950f4a51ca0f5edee5d88966 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:37 +0200 Subject: [PATCH 19/23] iotests.py: Add qemu_nbd function Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/iotests.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 3329bc1721..5a2678fcc2 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')] if os.environ.get('QEMU_IO_OPTIONS'): qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ') +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')] +if os.environ.get('QEMU_NBD_OPTIONS'): + qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ') + qemu_prog = os.environ.get('QEMU_PROG', 'qemu') qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ') @@ -87,6 +91,10 @@ def qemu_io(*args): sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args))) return subp.communicate()[0] +def qemu_nbd(*args): + '''Run qemu-nbd in daemon mode and return the parent's exit code''' + return subprocess.call(qemu_nbd_args + ['--fork'] + list(args)) + def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt): '''Return True if two image files are identical''' return qemu_img('compare', '-f', fmt1, From 5fcbdf508a9b5962d2e609b95364e1aae8797a4c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:38 +0200 Subject: [PATCH 20/23] iotests.py: Allow concurrent qemu instances By adding an optional suffix to the files used for communication with a VM, we can launch multiple VM instances concurrently. Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/iotests.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 5a2678fcc2..c589deb39e 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -140,8 +140,10 @@ def log(msg, filters=[]): class VM(qtest.QEMUQtestMachine): '''A QEMU VM''' - def __init__(self): - super(VM, self).__init__(qemu_prog, qemu_opts, test_dir=test_dir, + def __init__(self, path_suffix=''): + name = "qemu%s-%d" % (path_suffix, os.getpid()) + super(VM, self).__init__(qemu_prog, qemu_opts, name=name, + test_dir=test_dir, socket_scm_helper=socket_scm_helper) if debug: self._debug = True From d35172b425a032d7c9cc2453556d73a54e4ecfa1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:39 +0200 Subject: [PATCH 21/23] socket_scm_helper: Accept fd directly This gives us more freedom about the fd that is passed to qemu, allowing us to e.g. pass sockets. Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/socket_scm_helper.c | 29 ++++++++++++++++---------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c index 80cadf43bc..eb76d31aa9 100644 --- a/tests/qemu-iotests/socket_scm_helper.c +++ b/tests/qemu-iotests/socket_scm_helper.c @@ -60,7 +60,7 @@ static int send_fd(int fd, int fd_to_send) } /* Convert string to fd number. */ -static int get_fd_num(const char *fd_str) +static int get_fd_num(const char *fd_str, bool silent) { int sock; char *err; @@ -68,12 +68,16 @@ static int get_fd_num(const char *fd_str) errno = 0; sock = strtol(fd_str, &err, 10); if (errno) { - fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n", - strerror(errno)); + if (!silent) { + fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n", + strerror(errno)); + } return -1; } if (!*fd_str || *err || sock < 0) { - fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str); + if (!silent) { + fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str); + } return -1; } @@ -104,18 +108,21 @@ int main(int argc, char **argv, char **envp) } - sock = get_fd_num(argv[1]); + sock = get_fd_num(argv[1], false); if (sock < 0) { return EXIT_FAILURE; } - /* Now only open a file in readonly mode for test purpose. If more precise - control is needed, use python script in file operation, which is - supposed to fork and exec this program. */ - fd = open(argv[2], O_RDONLY); + fd = get_fd_num(argv[2], true); if (fd < 0) { - fprintf(stderr, "Failed to open file '%s'\n", argv[2]); - return EXIT_FAILURE; + /* Now only open a file in readonly mode for test purpose. If more + precise control is needed, use python script in file operation, which + is supposed to fork and exec this program. */ + fd = open(argv[2], O_RDONLY); + if (fd < 0) { + fprintf(stderr, "Failed to open file '%s'\n", argv[2]); + return EXIT_FAILURE; + } } ret = send_fd(sock, fd); From e07375f5523289d5fc279e0b7d3e93e51a28e2e3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:40 +0200 Subject: [PATCH 22/23] iotests: Add assert_json_filename_equal() method Since the order of keys in JSON filenames is not necessarily fixed, they should not be compared to fixed strings. This method takes a Python dict as a reference, parses a given JSON filename and compares both. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index c589deb39e..1f30cfcc75 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -222,6 +222,19 @@ class QMPTestCase(unittest.TestCase): self.fail('invalid index "%s" in path "%s" in "%s"' % (idx, path, str(d))) return d + def flatten_qmp_object(self, obj, output=None, basestr=''): + if output is None: + output = dict() + if isinstance(obj, list): + for i in range(len(obj)): + self.flatten_qmp_object(obj[i], output, basestr + str(i) + '.') + elif isinstance(obj, dict): + for key in obj: + self.flatten_qmp_object(obj[key], output, basestr + key + '.') + else: + output[basestr[:-1]] = obj # Strip trailing '.' + return output + def assert_qmp_absent(self, d, path): try: result = self.dictpath(d, path) @@ -252,6 +265,13 @@ class QMPTestCase(unittest.TestCase): self.assertTrue(False, "Cannot find %s %s in result:\n%s" % \ (node_name, file_name, result)) + def assert_json_filename_equal(self, json_filename, reference): + '''Asserts that the given filename is a json: filename and that its + content is equal to the given reference object''' + self.assertEqual(json_filename[:5], 'json:') + self.assertEqual(self.flatten_qmp_object(json.loads(json_filename[5:])), + self.flatten_qmp_object(reference)) + def cancel_and_wait(self, drive='drive0', force=False, resume=False): '''Cancel a block job and wait for it to finish, returning the event''' result = self.vm.qmp('block-job-cancel', device=drive, force=force) From b74fc7f78e0dd54fbae67d46552cebf81b59ae9f Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:41 +0200 Subject: [PATCH 23/23] iotests: Add test for NBD's blockdev-add interface Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/147 | 195 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/147.out | 5 + tests/qemu-iotests/group | 1 + 3 files changed, 201 insertions(+) create mode 100755 tests/qemu-iotests/147 create mode 100644 tests/qemu-iotests/147.out diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147 new file mode 100755 index 0000000000..45469c911e --- /dev/null +++ b/tests/qemu-iotests/147 @@ -0,0 +1,195 @@ +#!/usr/bin/env python +# +# Test case for NBD's blockdev-add interface +# +# Copyright (C) 2016 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import socket +import stat +import time +import iotests +from iotests import cachemode, imgfmt, qemu_img, qemu_nbd + +NBD_PORT = 10811 + +test_img = os.path.join(iotests.test_dir, 'test.img') +unix_socket = os.path.join(iotests.test_dir, 'nbd.socket') + +class NBDBlockdevAddBase(iotests.QMPTestCase): + def blockdev_add_options(self, address, export=None): + options = { 'node-name': 'nbd-blockdev', + 'driver': 'raw', + 'file': { + 'driver': 'nbd', + 'server': address + } } + if export is not None: + options['file']['export'] = export + return options + + def client_test(self, filename, address, export=None): + bao = self.blockdev_add_options(address, export) + result = self.vm.qmp('blockdev-add', **bao) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-named-block-nodes') + for node in result['return']: + if node['node-name'] == 'nbd-blockdev': + if isinstance(filename, str): + self.assert_qmp(node, 'image/filename', filename) + else: + self.assert_json_filename_equal(node['image']['filename'], + filename) + break + + result = self.vm.qmp('x-blockdev-del', node_name='nbd-blockdev') + self.assert_qmp(result, 'return', {}) + + +class QemuNBD(NBDBlockdevAddBase): + def setUp(self): + qemu_img('create', '-f', iotests.imgfmt, test_img, '64k') + self.vm = iotests.VM() + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(test_img) + try: + os.remove(unix_socket) + except OSError: + pass + + def _server_up(self, *args): + self.assertEqual(qemu_nbd('-f', imgfmt, test_img, *args), 0) + + def test_inet(self): + self._server_up('-p', str(NBD_PORT)) + address = { 'type': 'inet', + 'data': { + 'host': 'localhost', + 'port': str(NBD_PORT) + } } + self.client_test('nbd://localhost:%i' % NBD_PORT, address) + + def test_unix(self): + self._server_up('-k', unix_socket) + address = { 'type': 'unix', + 'data': { 'path': unix_socket } } + self.client_test('nbd+unix://?socket=' + unix_socket, address) + + +class BuiltinNBD(NBDBlockdevAddBase): + def setUp(self): + qemu_img('create', '-f', iotests.imgfmt, test_img, '64k') + self.vm = iotests.VM() + self.vm.launch() + self.server = iotests.VM('.server') + self.server.add_drive_raw('if=none,id=nbd-export,' + + 'file=%s,' % test_img + + 'format=%s,' % imgfmt + + 'cache=%s' % cachemode) + self.server.launch() + + def tearDown(self): + self.vm.shutdown() + self.server.shutdown() + os.remove(test_img) + try: + os.remove(unix_socket) + except OSError: + pass + + def _server_up(self, address): + result = self.server.qmp('nbd-server-start', addr=address) + self.assert_qmp(result, 'return', {}) + + result = self.server.qmp('nbd-server-add', device='nbd-export') + self.assert_qmp(result, 'return', {}) + + def _server_down(self): + result = self.server.qmp('nbd-server-stop') + self.assert_qmp(result, 'return', {}) + + def test_inet(self): + address = { 'type': 'inet', + 'data': { + 'host': 'localhost', + 'port': str(NBD_PORT) + } } + self._server_up(address) + self.client_test('nbd://localhost:%i/nbd-export' % NBD_PORT, + address, 'nbd-export') + self._server_down() + + def test_inet6(self): + address = { 'type': 'inet', + 'data': { + 'host': '::1', + 'port': str(NBD_PORT), + 'ipv4': False, + 'ipv6': True + } } + filename = { 'driver': 'raw', + 'file': { + 'driver': 'nbd', + 'export': 'nbd-export', + 'server': address + } } + self._server_up(address) + self.client_test(filename, address, 'nbd-export') + self._server_down() + + def test_unix(self): + address = { 'type': 'unix', + 'data': { 'path': unix_socket } } + self._server_up(address) + self.client_test('nbd+unix:///nbd-export?socket=' + unix_socket, + address, 'nbd-export') + self._server_down() + + def test_fd(self): + self._server_up({ 'type': 'unix', + 'data': { 'path': unix_socket } }) + + sockfd = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sockfd.connect(unix_socket) + + result = self.vm.send_fd_scm(str(sockfd.fileno())) + self.assertEqual(result, 0, 'Failed to send socket FD') + + result = self.vm.qmp('getfd', fdname='nbd-fifo') + self.assert_qmp(result, 'return', {}) + + address = { 'type': 'fd', + 'data': { 'str': 'nbd-fifo' } } + filename = { 'driver': 'raw', + 'file': { + 'driver': 'nbd', + 'export': 'nbd-export', + 'server': address + } } + self.client_test(filename, address, 'nbd-export') + + self._server_down() + + +if __name__ == '__main__': + # Need to support image creation + iotests.main(supported_fmts=['vpc', 'parallels', 'qcow', 'vdi', 'qcow2', + 'vmdk', 'raw', 'vhdx', 'qed']) diff --git a/tests/qemu-iotests/147.out b/tests/qemu-iotests/147.out new file mode 100644 index 0000000000..3f8a935a08 --- /dev/null +++ b/tests/qemu-iotests/147.out @@ -0,0 +1,5 @@ +...... +---------------------------------------------------------------------- +Ran 6 tests + +OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 7eb17707a2..d7d50cfe42 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -149,6 +149,7 @@ 144 rw auto quick 145 auto quick 146 auto quick +147 auto 148 rw auto quick 149 rw auto sudo 150 rw auto quick