From be07a889816b72c6e73b7649afe563590156ff9c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 12:56:14 +0200 Subject: 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 1a724a8..96bb634 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)); } -int blk_flush(BlockBackend *blk) +static void blk_flush_entry(void *opaque) { - if (!blk_is_available(blk)) { - return -ENOMEDIUM; - } + BlkRwCo *rwco = opaque; + rwco->ret = blk_co_flush(rwco->blk); +} - return bdrv_flush(blk_bs(blk)); +int blk_flush(BlockBackend *blk) +{ + return blk_prw(blk, 0, NULL, 0, blk_flush_entry, 0); } void blk_drain(BlockBackend *blk) -- cgit v1.1 From 8c2e3dd55f54ba00fd0893a535c09b5fcd7a877b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 12:56:14 +0200 Subject: 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 96bb634..39336de 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); } -int blk_pdiscard(BlockBackend *blk, int64_t offset, int count) +static void blk_pdiscard_entry(void *opaque) { - int ret = blk_check_byte_request(blk, offset, count); - if (ret < 0) { - return ret; - } + BlkRwCo *rwco = opaque; + rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size); +} - return bdrv_pdiscard(blk_bs(blk), offset, count); +int blk_pdiscard(BlockBackend *blk, int64_t offset, int count) +{ + return blk_prw(blk, offset, NULL, count, blk_pdiscard_entry, 0); } int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf, -- cgit v1.1 From 7381e95cc2c33b589c94a857dff21bf2016a08b7 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:40:22 +0200 Subject: 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 b136c89..ff93ba1 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 05fa13c..aff8a96 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 107c603..99a15a6 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); -- cgit v1.1 From 48af776a5b85ad2dc6124d3d0fb210ca6b98d32c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 12:56:14 +0200 Subject: 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 39336de..c53ca30 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; + + iov = (struct iovec) { + .iov_base = buf, + .iov_len = 0, + }; + qemu_iovec_init_external(&qiov, &iov, 1); - return bdrv_aio_ioctl(blk_bs(blk), req, buf, cb, opaque); + 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 ff93ba1..7c119d5 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 99a15a6..e06db62 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 b07159b..6444e41 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); -- cgit v1.1 From 0d4377b3ea97cc28c71642c92c2cfa8cdb1cc8bd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:50:12 +0200 Subject: 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 f481e57..247e47b 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)) { + return false; + } - 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)) { + 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; -- cgit v1.1 From 61b2450414ee052c8f2561e999852fad0534899e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:40:22 +0200 Subject: 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 7c119d5..35fdcca 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 e06db62..e0a54aa 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); -- cgit v1.1 From 16a389dc9ecc05e9d8d6ebd3eff6dc98158523e0 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 15:07:27 +0200 Subject: 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 35fdcca..370c7d8 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 3e79228..e96e9ad 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; -- cgit v1.1 From 151a2930c437e9f09f6b1c21c99ccabfd5dba9c7 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 15:09:36 +0200 Subject: 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 588d408..fc16ec1 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 }; -- cgit v1.1 From cbc14ac9c3e199df759e2847b2301e2b990a8537 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 20 Oct 2016 14:40:22 +0200 Subject: 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 370c7d8..79cbbdf 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 e0a54aa..398a050 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); -- cgit v1.1 From e5b77eec91eb03101d53d8ce4a5d03e40b5d1000 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 27 Oct 2016 13:16:56 +0200 Subject: 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 126bd67..3213765 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 -- cgit v1.1 From 82d73014a9111c53fefb7e7954c9e370d23f7569 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:29 +0200 Subject: 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 6bc06d6..ce7c14f 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 408d613..9e584fe 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 ec6d222..6395a30 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 -- cgit v1.1 From 442045cbce010688eed2b8ede6141e982c6d6b23 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:30 +0200 Subject: 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 ce7c14f..eaca33c 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); -- cgit v1.1 From 7edca33804f9b6948a65c04f969facf02a59e6fe Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:31 +0200 Subject: 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 eaca33c..c77a969 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))); -- cgit v1.1 From fcfcd8ffccd81b6fc13a730e2a75cefc5d1eb752 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:32 +0200 Subject: 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 c77a969..c539fb5 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; -- cgit v1.1 From 48c38e0b8d2d71ec82928bd4b4136a941d39de3c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:33 +0200 Subject: 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 c539fb5..cdab20f 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; } -- cgit v1.1 From 491d6c7c4e4793ae5b009e120139ad62f56b5a35 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:34 +0200 Subject: 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 | 175 +++++++++++++++++++++++++++--------------- tests/qemu-iotests/051.out | 4 +- tests/qemu-iotests/051.pc.out | 4 +- 3 files changed, 117 insertions(+), 66 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index cdab20f..a778692 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 (!path && !host && !port) { + return true; + } - 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"); + 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; } - return NULL; } - if (s->port && !s->host) { - error_setg(errp, "port may not be used without host"); - return NULL; + + 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))); } - saddr = g_new0(SocketAddress, 1); + return true; +} - 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); - } +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; } - s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX; + crumpled_addr = qdict_crumple(addr, errp); + if (!crumpled_addr) { + goto done; + } - s->export = g_strdup(qemu_opt_get(opts, "export")); + 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; + +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; } + /* 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. */ - saddr = nbd_config(s, opts, errp); - if (!saddr) { + 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 9e584fe..42bf416 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 6395a30..603bb76 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 -- cgit v1.1 From f84d431b86f6f00a8fcefb44af08243e70466acb Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:35 +0200 Subject: 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 a778692..8ef1438 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); } -- cgit v1.1 From 6b02b1f0a4a5d887e6e773e9f13fffcb2adfdf0d Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:36 +0200 Subject: 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 97b1205..cd1fa7b 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 @@ -2220,6 +2221,24 @@ '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 # # Options for creating a block device. Many options are available for all @@ -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', -- cgit v1.1 From bec87774c24aac5b950f4a51ca0f5edee5d88966 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:37 +0200 Subject: 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 3329bc1..5a2678f 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, -- cgit v1.1 From 5fcbdf508a9b5962d2e609b95364e1aae8797a4c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:38 +0200 Subject: 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 5a2678f..c589deb 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 -- cgit v1.1 From d35172b425a032d7c9cc2453556d73a54e4ecfa1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:39 +0200 Subject: 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 80cadf4..eb76d31 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); -- cgit v1.1 From e07375f5523289d5fc279e0b7d3e93e51a28e2e3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:40 +0200 Subject: 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 c589deb..1f30cfc 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) -- cgit v1.1 From b74fc7f78e0dd54fbae67d46552cebf81b59ae9f Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 25 Oct 2016 15:11:41 +0200 Subject: 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 0000000..45469c9 --- /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 0000000..3f8a935 --- /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 7eb1770..d7d50cf 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 -- cgit v1.1