Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

在 2017/7/4 22:04, Christian Borntraeger 写道:

On 07/04/2017 03:23 PM, QingFeng Hao wrote:

This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
Currently we don't have an equivalent to "memory: emulate ioeventfd"
for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
skipping iothread arguments.

I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao 
Reviewed-by: Cornelia Huck 

I will take it via the s390-next tree.

thanks applied.

Ok, thanks.

---
  hw/s390x/virtio-ccw.c | 2 +-
  target/s390x/cpu.h| 6 +-
  2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, 
Error **errp)
  sch->cssid, sch->ssid, sch->schid, sch->devno,
  ccw_dev->devno.valid ? "user-configured" : "auto-configured");

-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
  dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
  }

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9faca04b52..bdb9bdbc9d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int 
s390_assign_subch_ioeventfd(EventNotifier *notifier,
uint32_t sch_id, int vq,
bool assign)
  {
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
  }

  static inline void s390_crypto_reset(void)



--
Regards
QingFeng Hao

[Qemu-block] [PATCH 30/35] block-backend: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 include/sysemu/block-backend.h |  4 ++--
 block/block-backend.c  | 36 
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 1e05281fff..2f967037af 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -165,8 +165,8 @@ 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);
-int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
-int blk_co_flush(BlockBackend *blk);
+int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
+int coroutine_fn blk_co_flush(BlockBackend *blk);
 int blk_flush(BlockBackend *blk);
 int blk_commit_all(void);
 void blk_drain(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 56fc0a4d1e..a48aa4f900 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1032,7 +1032,8 @@ typedef struct BlkRwCo {
 BdrvRequestFlags flags;
 } BlkRwCo;
 
-static void blk_read_entry(void *opaque)
+static void coroutine_fn
+blk_read_entry(void *opaque)
 {
 BlkRwCo *rwco = opaque;
 
@@ -1040,7 +1041,8 @@ static void blk_read_entry(void *opaque)
   rwco->qiov, rwco->flags);
 }
 
-static void blk_write_entry(void *opaque)
+static void coroutine_fn
+blk_write_entry(void *opaque)
 {
 BlkRwCo *rwco = opaque;
 
@@ -1195,7 +1197,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, 
int64_t offset, int bytes,
 return >common;
 }
 
-static void blk_aio_read_entry(void *opaque)
+static void coroutine_fn
+blk_aio_read_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = >rwco;
@@ -1206,7 +1209,8 @@ static void blk_aio_read_entry(void *opaque)
 blk_aio_complete(acb);
 }
 
-static void blk_aio_write_entry(void *opaque)
+static void coroutine_fn
+blk_aio_write_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = >rwco;
@@ -1288,7 +1292,8 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t 
offset,
 blk_aio_write_entry, flags, cb, opaque);
 }
 
-static void blk_aio_flush_entry(void *opaque)
+static void coroutine_fn
+blk_aio_flush_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = >rwco;
@@ -1303,7 +1308,8 @@ 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)
+static void coroutine_fn
+blk_aio_pdiscard_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = >rwco;
@@ -1339,7 +1345,8 @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int 
req, void *buf)
 return bdrv_co_ioctl(blk_bs(blk), req, buf);
 }
 
-static void blk_ioctl_entry(void *opaque)
+static void coroutine_fn
+blk_ioctl_entry(void *opaque)
 {
 BlkRwCo *rwco = opaque;
 rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
@@ -1351,7 +1358,8 @@ 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)
+static void coroutine_fn
+blk_aio_ioctl_entry(void *opaque)
 {
 BlkAioEmAIOCB *acb = opaque;
 BlkRwCo *rwco = >rwco;
@@ -1376,7 +1384,8 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned 
long int req, void *buf,
 return blk_aio_prwv(blk, req, 0, , blk_aio_ioctl_entry, 0, cb, 
opaque);
 }
 
-int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
+int coroutine_fn
+blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
 {
 int ret = blk_check_byte_request(blk, offset, bytes);
 if (ret < 0) {
@@ -1386,7 +1395,8 @@ int blk_co_pdiscard(BlockBackend *blk, int64_t offset, 
int bytes)
 return bdrv_co_pdiscard(blk_bs(blk), offset, bytes);
 }
 
-int blk_co_flush(BlockBackend *blk)
+int coroutine_fn
+blk_co_flush(BlockBackend *blk)
 {
 if (!blk_is_available(blk)) {
 return -ENOMEDIUM;
@@ -1395,7 +1405,8 @@ int blk_co_flush(BlockBackend *blk)
 return bdrv_co_flush(blk_bs(blk));
 }
 
-static void blk_flush_entry(void *opaque)
+static void coroutine_fn
+blk_flush_entry(void *opaque)
 {
 BlkRwCo *rwco = opaque;
 rwco->ret = blk_co_flush(rwco->blk);
@@ -1785,7 +1796,8 @@ int blk_truncate(BlockBackend *blk, int64_t offset, Error 
**errp)
 return bdrv_truncate(blk->root, offset, errp);
 }
 
-static void blk_pdiscard_entry(void *opaque)
+static void coroutine_fn
+blk_pdiscard_entry(void *opaque)
 {
 BlkRwCo *rwco = opaque;
 rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size);
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 35/35] vpc: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/vpc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vpc.c b/block/vpc.c
index 4240ba9d1c..1b4aba20bd 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -872,7 +872,8 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t 
*buf,
 return ret;
 }
 
-static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vpc_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 uint8_t buf[1024];
 VHDFooter *footer = (VHDFooter *) buf;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 29/35] block: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/blkdebug.c  | 15 ++-
 block/blkverify.c |  3 ++-
 block/io.c|  9 ++---
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index a1b24b9b0d..d55e2e69c8 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -483,7 +483,8 @@ out:
 return ret;
 }
 
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
+static int coroutine_fn
+rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
 {
 BDRVBlkdebugState *s = bs->opaque;
 BlkdebugRule *rule = NULL;
@@ -563,7 +564,8 @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
 return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
 }
 
-static int blkdebug_co_flush(BlockDriverState *bs)
+static int coroutine_fn
+blkdebug_co_flush(BlockDriverState *bs)
 {
 int err = rule_check(bs, 0, 0);
 
@@ -656,7 +658,8 @@ static void blkdebug_close(BlockDriverState *bs)
 g_free(s->config_file);
 }
 
-static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
+static void coroutine_fn
+suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
 {
 BDRVBlkdebugState *s = bs->opaque;
 BlkdebugSuspendedReq r;
@@ -681,7 +684,8 @@ static void suspend_request(BlockDriverState *bs, 
BlkdebugRule *rule)
 g_free(r.tag);
 }
 
-static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
+static bool coroutine_fn
+process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
 bool injected)
 {
 BDRVBlkdebugState *s = bs->opaque;
@@ -712,7 +716,8 @@ static bool process_rule(BlockDriverState *bs, struct 
BlkdebugRule *rule,
 return injected;
 }
 
-static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
+static void coroutine_fn
+blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
 {
 BDRVBlkdebugState *s = bs->opaque;
 struct BlkdebugRule *rule, *next;
diff --git a/block/blkverify.c b/block/blkverify.c
index 06369f9eac..d0c946173a 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -255,7 +255,8 @@ blkverify_co_pwritev(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
 return blkverify_co_prwv(bs, , offset, bytes, qiov, qiov, flags, true);
 }
 
-static int blkverify_co_flush(BlockDriverState *bs)
+static int coroutine_fn
+blkverify_co_flush(BlockDriverState *bs)
 {
 BDRVBlkverifyState *s = bs->opaque;
 
diff --git a/block/io.c b/block/io.c
index 14b88c8609..a53a86df3e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -366,7 +366,8 @@ void bdrv_drain_all(void)
  *
  * This function should be called when a tracked request is completing.
  */
-static void tracked_request_end(BdrvTrackedRequest *req)
+static void coroutine_fn
+tracked_request_end(BdrvTrackedRequest *req)
 {
 if (req->serialising) {
 atomic_dec(>bs->serialising_in_flight);
@@ -381,7 +382,8 @@ static void tracked_request_end(BdrvTrackedRequest *req)
 /**
  * Add an active request to the tracked requests list
  */
-static void tracked_request_begin(BdrvTrackedRequest *req,
+static void coroutine_fn
+tracked_request_begin(BdrvTrackedRequest *req,
   BlockDriverState *bs,
   int64_t offset,
   unsigned int bytes,
@@ -2430,7 +2432,8 @@ int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, 
int bytes)
 return rwco.ret;
 }
 
-int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
+int coroutine_fn
+bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
 {
 BlockDriver *drv = bs->drv;
 CoroutineIOCompletion co = {
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 31/35] parallels: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/parallels.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index 8be46a7d48..213e42b9d2 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -472,7 +472,8 @@ static int parallels_check(BlockDriverState *bs, 
BdrvCheckResult *res,
 }
 
 
-static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+parallels_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int64_t total_size, cl_size;
 uint8_t tmp[BDRV_SECTOR_SIZE];
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 33/35] vdi: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/vdi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vdi.c b/block/vdi.c
index 79af47763b..53cd7f64d8 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -716,7 +716,8 @@ vdi_co_pwritev(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
 return ret;
 }
 
-static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vdi_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int ret = 0;
 uint64_t bytes = 0;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 22/35] sheepdog: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/sheepdog.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 83bc43dde4..64ff275db9 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -481,7 +481,8 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, 
SheepdogAIOCB *acb,
 return aio_req;
 }
 
-static void wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB 
*acb)
+static void coroutine_fn
+wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
 {
 SheepdogAIOCB *cb;
 
@@ -494,7 +495,8 @@ retry:
 }
 }
 
-static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
+static void coroutine_fn
+sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
  QEMUIOVector *qiov, int64_t sector_num, int 
nb_sectors,
  int type)
 {
@@ -1954,7 +1956,8 @@ static int parse_block_size_shift(BDRVSheepdogState *s, 
QemuOpts *opt)
 return 0;
 }
 
-static int sd_create(const char *filename, QemuOpts *opts,
+static int coroutine_fn
+sd_create(const char *filename, QemuOpts *opts,
  Error **errp)
 {
 Error *err = NULL;
@@ -2431,7 +2434,8 @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB 
*acb)
 }
 }
 
-static void sd_aio_complete(SheepdogAIOCB *acb)
+static void coroutine_fn
+sd_aio_complete(SheepdogAIOCB *acb)
 {
 if (acb->aiocb_type == AIOCB_FLUSH_CACHE) {
 return;
@@ -2905,7 +2909,8 @@ cleanup:
 return ret;
 }
 
-static int sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
 {
 BDRVSheepdogState *s = bs->opaque;
@@ -2920,7 +2925,8 @@ static int sd_save_vmstate(BlockDriverState *bs, 
QEMUIOVector *qiov,
 return ret;
 }
 
-static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
 {
 BDRVSheepdogState *s = bs->opaque;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 34/35] vhdx: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/vhdx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 8b270b57c9..56b54f3ed7 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1787,7 +1787,8 @@ exit:
  *. ~ --- ~  ~  ~ ---.
  *   1MB
  */
-static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int ret = 0;
 uint64_t image_size = (uint64_t) 2 * GiB;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 26/35] iscsi: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/iscsi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 54067e2620..e16311cb4a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1005,7 +1005,8 @@ static void iscsi_ioctl_handle_emulated(IscsiAIOCB *acb, 
int req, void *buf)
 qemu_bh_schedule(acb->bh);
 }
 
-static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+iscsi_aio_ioctl(BlockDriverState *bs,
 unsigned long int req, void *buf,
 BlockCompletionFunc *cb, void *opaque)
 {
@@ -2107,7 +2108,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t 
offset, Error **errp)
 return 0;
 }
 
-static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int ret = 0;
 int64_t total_size = 0;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 25/35] mirror: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/mirror.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 68744a17e8..2f0a9946d9 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -224,7 +224,8 @@ static int mirror_cow_align(MirrorBlockJob *s,
 return ret;
 }
 
-static inline void mirror_wait_for_io(MirrorBlockJob *s)
+static inline void coroutine_fn
+mirror_wait_for_io(MirrorBlockJob *s)
 {
 assert(!s->waiting_for_io);
 s->waiting_for_io = true;
@@ -239,7 +240,8 @@ static inline void mirror_wait_for_io(MirrorBlockJob *s)
  *  (new_end - sector_num) if tail is rounded up or down due to
  *  alignment or buffer limit.
  */
-static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
+static int coroutine_fn
+mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
   int nb_sectors)
 {
 BlockBackend *source = s->common.blk;
@@ -490,7 +492,8 @@ static void mirror_free_init(MirrorBlockJob *s)
  * mirror_resume() because mirror_run() will begin iterating again
  * when the job is resumed.
  */
-static void mirror_wait_for_all_io(MirrorBlockJob *s)
+static void coroutine_fn
+mirror_wait_for_all_io(MirrorBlockJob *s)
 {
 while (s->in_flight > 0) {
 mirror_wait_for_io(s);
@@ -605,7 +608,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
 bdrv_unref(src);
 }
 
-static void mirror_throttle(MirrorBlockJob *s)
+static void coroutine_fn
+mirror_throttle(MirrorBlockJob *s)
 {
 int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 
@@ -984,7 +988,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
 block_job_enter(>common);
 }
 
-static void mirror_pause(BlockJob *job)
+static void coroutine_fn
+mirror_pause(BlockJob *job)
 {
 MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
 
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 32/35] qed: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/qed.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qed.c b/block/qed.c
index 385381a78a..dd2859a1c9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -622,7 +622,8 @@ out:
 return ret;
 }
 
-static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 uint64_t image_size = 0;
 uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 17/35] curl: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/curl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 2a244e2439..d3719dc086 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -855,7 +855,8 @@ out_noclean:
 return -EINVAL;
 }
 
-static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
+static void coroutine_fn
+curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
 {
 CURLState *state;
 int running;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 27/35] file-posix: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/file-posix.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 3927fabf06..adafbbb6a0 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1483,7 +1483,8 @@ static int aio_worker(void *arg)
 return ret;
 }
 
-static int paio_submit_co(BlockDriverState *bs, int fd,
+static int coroutine_fn
+paio_submit_co(BlockDriverState *bs, int fd,
   int64_t offset, QEMUIOVector *qiov,
   int bytes, int type)
 {
@@ -1599,7 +1600,8 @@ static void raw_aio_unplug(BlockDriverState *bs)
 #endif
 }
 
-static BlockAIOCB *raw_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+raw_aio_flush(BlockDriverState *bs,
 BlockCompletionFunc *cb, void *opaque)
 {
 BDRVRawState *s = bs->opaque;
@@ -1835,7 +1837,8 @@ static int64_t 
raw_get_allocated_file_size(BlockDriverState *bs)
 return (int64_t)st.st_blocks * 512;
 }
 
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int fd;
 int result = 0;
@@ -2526,7 +2529,8 @@ hdev_open_Mac_error:
 
 #if defined(__linux__)
 
-static BlockAIOCB *hdev_aio_ioctl(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+hdev_aio_ioctl(BlockDriverState *bs,
 unsigned long int req, void *buf,
 BlockCompletionFunc *cb, void *opaque)
 {
@@ -2592,7 +2596,8 @@ static coroutine_fn int 
hdev_co_pwrite_zeroes(BlockDriverState *bs,
 return -ENOTSUP;
 }
 
-static int hdev_create(const char *filename, QemuOpts *opts,
+static int coroutine_fn
+hdev_create(const char *filename, QemuOpts *opts,
Error **errp)
 {
 int fd;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 12/35] raw: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/raw-format.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index 0d185fe41b..402d3b9fba 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -361,7 +361,8 @@ static void raw_lock_medium(BlockDriverState *bs, bool 
locked)
 bdrv_lock_medium(bs->file->bs, locked);
 }
 
-static int raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
+static int coroutine_fn
+raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
 {
 BDRVRawState *s = bs->opaque;
 if (s->offset || s->has_size) {
@@ -375,7 +376,8 @@ static int raw_has_zero_init(BlockDriverState *bs)
 return bdrv_has_zero_init(bs->file->bs);
 }
 
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 return bdrv_create_file(filename, opts, errp);
 }
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 19/35] nfs: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/nfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/nfs.c b/block/nfs.c
index c3c5de0113..3f393a95a4 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -679,7 +679,8 @@ static QemuOptsList nfs_create_opts = {
 }
 };
 
-static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
 {
 int ret = 0;
 int64_t total_size = 0;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 16/35] crypto: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/crypto.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/crypto.c b/block/crypto.c
index 10e5ddccaa..0e30a4ea06 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -568,7 +568,8 @@ static int block_crypto_open_luks(BlockDriverState *bs,
  bs, options, flags, errp);
 }
 
-static int block_crypto_create_luks(const char *filename,
+static int coroutine_fn
+block_crypto_create_luks(const char *filename,
 QemuOpts *opts,
 Error **errp)
 {
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 09/35] block: bdrv_create() and bdrv_debug_event() are coroutine_fn

[Marc-André Lureau]

Called from coroutine.

Signed-off-by: Marc-André Lureau 
---
 include/block/block_int.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 93eb2a9528..a183c72b7c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -126,7 +126,7 @@ struct BlockDriver {
 int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
   Error **errp);
 void (*bdrv_close)(BlockDriverState *bs);
-int (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp);
+int coroutine_fn (*bdrv_create)(const char *filename, QemuOpts *opts, 
Error **errp);
 int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
 int (*bdrv_make_empty)(BlockDriverState *bs);
 
@@ -267,7 +267,7 @@ struct BlockDriver {
   BlockDriverAmendStatusCB *status_cb,
   void *cb_opaque);
 
-void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
+void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent 
event);
 
 /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
 int (*bdrv_debug_breakpoint)(BlockDriverState *bs, const char *event,
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 13/35] nbd: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/nbd-client.h | 10 +-
 block/nbd-client.c | 24 
 block/nbd.c|  3 ++-
 nbd/server.c   |  3 ++-
 4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 49636bc621..473d1f88fd 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -42,13 +42,13 @@ int nbd_client_init(BlockDriverState *bs,
 Error **errp);
 void nbd_client_close(BlockDriverState *bs);
 
-int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
-int nbd_client_co_flush(BlockDriverState *bs);
-int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, 
int bytes);
+int coroutine_fn nbd_client_co_flush(BlockDriverState *bs);
+int coroutine_fn nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
   uint64_t bytes, QEMUIOVector *qiov, int flags);
-int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+int coroutine_fn nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t 
offset,
 int bytes, BdrvRequestFlags flags);
-int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
  uint64_t bytes, QEMUIOVector *qiov, int flags);
 
 void nbd_client_detach_aio_context(BlockDriverState *bs);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 02e928142e..63c0210c37 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -111,7 +111,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 s->read_reply_co = NULL;
 }
 
-static int nbd_co_send_request(BlockDriverState *bs,
+static int coroutine_fn
+nbd_co_send_request(BlockDriverState *bs,
NBDRequest *request,
QEMUIOVector *qiov)
 {
@@ -158,7 +159,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
 return rc;
 }
 
-static void nbd_co_receive_reply(NBDClientSession *s,
+static void coroutine_fn
+nbd_co_receive_reply(NBDClientSession *s,
  NBDRequest *request,
  NBDReply *reply,
  QEMUIOVector *qiov)
@@ -185,7 +187,8 @@ static void nbd_co_receive_reply(NBDClientSession *s,
 }
 }
 
-static void nbd_coroutine_end(BlockDriverState *bs,
+static void coroutine_fn
+nbd_coroutine_end(BlockDriverState *bs,
   NBDRequest *request)
 {
 NBDClientSession *s = nbd_get_client_session(bs);
@@ -204,7 +207,8 @@ static void nbd_coroutine_end(BlockDriverState *bs,
 qemu_co_mutex_unlock(>send_mutex);
 }
 
-int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
  uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
@@ -229,7 +233,8 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t 
offset,
 return -reply.error;
 }
 
-int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
   uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
@@ -258,7 +263,8 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t 
offset,
 return -reply.error;
 }
 
-int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+int coroutine_fn
+nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
 int bytes, BdrvRequestFlags flags)
 {
 ssize_t ret;
@@ -292,7 +298,8 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, 
int64_t offset,
 return -reply.error;
 }
 
-int nbd_client_co_flush(BlockDriverState *bs)
+int coroutine_fn
+nbd_client_co_flush(BlockDriverState *bs)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 NBDRequest request = { .type = NBD_CMD_FLUSH };
@@ -316,7 +323,8 @@ int nbd_client_co_flush(BlockDriverState *bs)
 return -reply.error;
 }
 
-int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+int coroutine_fn
+nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 NBDRequest request = {
diff --git a/block/nbd.c b/block/nbd.c
index d529305330..8689b27d7d 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -465,7 +465,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 return ret;
 }
 
-static int nbd_co_flush(BlockDriverState *bs)
+static int coroutine_fn
+nbd_co_flush(BlockDriverState *bs)
 {
 return nbd_client_co_flush(bs);
 }
diff --git a/nbd/server.c 

[Qemu-block] [PATCH 24/35] null: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/null.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/block/null.c b/block/null.c
index 876f90965b..4c8afe16d7 100644
--- a/block/null.c
+++ b/block/null.c
@@ -167,7 +167,8 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState 
*bs,
 return >common;
 }
 
-static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_readv(BlockDriverState *bs,
   int64_t sector_num, QEMUIOVector *qiov,
   int nb_sectors,
   BlockCompletionFunc *cb,
@@ -182,7 +183,8 @@ static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
 return null_aio_common(bs, cb, opaque);
 }
 
-static BlockAIOCB *null_aio_writev(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_writev(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov,
int nb_sectors,
BlockCompletionFunc *cb,
@@ -191,7 +193,8 @@ static BlockAIOCB *null_aio_writev(BlockDriverState *bs,
 return null_aio_common(bs, cb, opaque);
 }
 
-static BlockAIOCB *null_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_flush(BlockDriverState *bs,
   BlockCompletionFunc *cb,
   void *opaque)
 {
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 23/35] ssh: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/ssh.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 52964416da..03a8ebe6f7 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -813,7 +813,8 @@ static QemuOptsList ssh_create_opts = {
 }
 };
 
-static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+ssh_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int r, ret;
 int64_t total_size = 0;
@@ -1029,7 +1030,8 @@ static coroutine_fn int ssh_co_readv(BlockDriverState *bs,
 return ret;
 }
 
-static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
+static int coroutine_fn
+ssh_write(BDRVSSHState *s, BlockDriverState *bs,
  int64_t offset, size_t size,
  QEMUIOVector *qiov)
 {
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 10/35] vmdk: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/vmdk.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 55581b03fe..f8422e8971 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1334,7 +1334,8 @@ static int64_t coroutine_fn 
vmdk_co_get_block_status(BlockDriverState *bs,
 return ret;
 }
 
-static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
+static int coroutine_fn
+vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
 int64_t offset_in_cluster, QEMUIOVector *qiov,
 uint64_t qiov_offset, uint64_t n_bytes,
 uint64_t offset)
@@ -1406,7 +1407,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t 
cluster_offset,
 return ret;
 }
 
-static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
+static int coroutine_fn
+vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
 int64_t offset_in_cluster, QEMUIOVector *qiov,
 int bytes)
 {
@@ -1551,7 +1553,8 @@ fail:
  *
  * Returns: error code with 0 for success.
  */
-static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov,
bool zeroed, bool zero_dry_run)
 {
@@ -1857,7 +1860,8 @@ static int filename_decompose(const char *filename, char 
*path, char *prefix,
 return VMDK_OK;
 }
 
-static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int idx = 0;
 BlockBackend *new_blk = NULL;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 20/35] quorum: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/quorum.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 55ba916655..b086d70daa 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -264,7 +264,8 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
 }
 }
 
-static void quorum_rewrite_entry(void *opaque)
+static void coroutine_fn
+quorum_rewrite_entry(void *opaque)
 {
 QuorumCo *co = opaque;
 QuorumAIOCB *acb = co->acb;
@@ -282,7 +283,8 @@ static void quorum_rewrite_entry(void *opaque)
 }
 }
 
-static bool quorum_rewrite_bad_versions(QuorumAIOCB *acb,
+static bool coroutine_fn
+quorum_rewrite_bad_versions(QuorumAIOCB *acb,
 QuorumVoteValue *value)
 {
 QuorumVoteVersion *version;
@@ -497,7 +499,8 @@ static int quorum_vote_error(QuorumAIOCB *acb)
 return ret;
 }
 
-static void quorum_vote(QuorumAIOCB *acb)
+static void coroutine_fn
+quorum_vote(QuorumAIOCB *acb)
 {
 bool quorum = true;
 int i, j, ret;
@@ -577,7 +580,8 @@ free_exit:
 quorum_free_vote_list(>votes);
 }
 
-static void read_quorum_children_entry(void *opaque)
+static void coroutine_fn
+read_quorum_children_entry(void *opaque)
 {
 QuorumCo *co = opaque;
 QuorumAIOCB *acb = co->acb;
@@ -605,7 +609,7 @@ static void read_quorum_children_entry(void *opaque)
 }
 }
 
-static int read_quorum_children(QuorumAIOCB *acb)
+static int coroutine_fn read_quorum_children(QuorumAIOCB *acb)
 {
 BDRVQuorumState *s = acb->bs->opaque;
 int i, ret;
@@ -648,7 +652,8 @@ static int read_quorum_children(QuorumAIOCB *acb)
 return ret;
 }
 
-static int read_fifo_child(QuorumAIOCB *acb)
+static int coroutine_fn
+read_fifo_child(QuorumAIOCB *acb)
 {
 BDRVQuorumState *s = acb->bs->opaque;
 int n, ret;
@@ -669,7 +674,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
 return ret;
 }
 
-static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
 uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
 BDRVQuorumState *s = bs->opaque;
@@ -689,7 +695,7 @@ static int quorum_co_preadv(BlockDriverState *bs, uint64_t 
offset,
 return ret;
 }
 
-static void write_quorum_entry(void *opaque)
+static void coroutine_fn write_quorum_entry(void *opaque)
 {
 QuorumCo *co = opaque;
 QuorumAIOCB *acb = co->acb;
@@ -715,7 +721,8 @@ static void write_quorum_entry(void *opaque)
 }
 }
 
-static int quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
  uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
 BDRVQuorumState *s = bs->opaque;
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 21/35] rbd: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/rbd.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 9da02cdceb..7b4d548cd2 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -348,7 +348,8 @@ static QemuOptsList runtime_opts = {
 },
 };
 
-static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 Error *local_err = NULL;
 int64_t bytes = 0;
@@ -861,7 +862,8 @@ failed:
 return NULL;
 }
 
-static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_readv(BlockDriverState *bs,
   int64_t sector_num,
   QEMUIOVector *qiov,
   int nb_sectors,
@@ -873,7 +875,8 @@ static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
  RBD_AIO_READ);
 }
 
-static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_writev(BlockDriverState *bs,
int64_t sector_num,
QEMUIOVector *qiov,
int nb_sectors,
@@ -886,7 +889,8 @@ static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
 }
 
 #ifdef LIBRBD_SUPPORTS_AIO_FLUSH
-static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_flush(BlockDriverState *bs,
   BlockCompletionFunc *cb,
   void *opaque)
 {
@@ -1063,7 +1067,8 @@ static int qemu_rbd_snap_list(BlockDriverState *bs,
 }
 
 #ifdef LIBRBD_SUPPORTS_DISCARD
-static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_pdiscard(BlockDriverState *bs,
  int64_t offset,
  int bytes,
  BlockCompletionFunc *cb,
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 15/35] backup: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 include/block/block_backup.h | 4 ++--
 block/backup.c   | 9 ++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/block/block_backup.h b/include/block/block_backup.h
index 8a759477a3..415cf8519d 100644
--- a/include/block/block_backup.h
+++ b/include/block/block_backup.h
@@ -27,12 +27,12 @@ typedef struct CowRequest {
 CoQueue wait_queue; /* coroutines blocked on this request */
 } CowRequest;
 
-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
+void coroutine_fn backup_wait_for_overlapping_requests(BlockJob *job, int64_t 
sector_num,
   int nb_sectors);
 void backup_cow_request_begin(CowRequest *req, BlockJob *job,
   int64_t sector_num,
   int nb_sectors);
-void backup_cow_request_end(CowRequest *req);
+void coroutine_fn backup_cow_request_end(CowRequest *req);
 
 void backup_do_checkpoint(BlockJob *job, Error **errp);
 
diff --git a/block/backup.c b/block/backup.c
index 5387fbd84e..58ddd80b3f 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -84,7 +84,8 @@ static void cow_request_begin(CowRequest *req, BackupBlockJob 
*job,
 }
 
 /* Forget about a completed request */
-static void cow_request_end(CowRequest *req)
+static void coroutine_fn
+cow_request_end(CowRequest *req)
 {
 QLIST_REMOVE(req, list);
 qemu_co_queue_restart_all(>wait_queue);
@@ -275,7 +276,8 @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
 bitmap_zero(backup_job->done_bitmap, len);
 }
 
-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
+void coroutine_fn
+backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
   int nb_sectors)
 {
 BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
@@ -304,7 +306,8 @@ void backup_cow_request_begin(CowRequest *req, BlockJob 
*job,
 cow_request_begin(req, backup_job, start, end);
 }
 
-void backup_cow_request_end(CowRequest *req)
+void coroutine_fn
+backup_cow_request_end(CowRequest *req)
 {
 cow_request_end(req);
 }
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 11/35] qcow2: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 block/qcow2.h |  6 --
 block/qcow.c  |  4 +++-
 block/qcow2-cluster.c | 11 +++
 block/qcow2.c | 15 ++-
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 87b15eb4aa..a32b47b7f6 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -550,14 +550,16 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
 
 int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
  unsigned int *bytes, uint64_t *cluster_offset);
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *host_offset,
QCowL2Meta **m);
 uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
  uint64_t offset,
  int compressed_size);
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int coroutine_fn
+qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
 int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
   uint64_t bytes, enum qcow2_discard_type type,
   bool full_discard);
diff --git a/block/qcow.c b/block/qcow.c
index 7bd94dcd46..d84ae7fb74 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -796,7 +796,9 @@ static void qcow_close(BlockDriverState *bs)
 error_free(s->migration_blocker);
 }
 
-static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
+
+static int coroutine_fn
+qcow_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int header_size, backing_filename_len, l1_size, shift, i;
 QCowHeader header;
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 3d341fd9cb..964d23aee8 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -761,7 +761,8 @@ uint64_t 
qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
 return cluster_offset;
 }
 
-static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
+static int coroutine_fn
+perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 {
 BDRVQcow2State *s = bs->opaque;
 Qcow2COWRegion *start = >cow_start;
@@ -890,7 +891,7 @@ fail:
 return ret;
 }
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+int coroutine_fn qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta 
*m)
 {
 BDRVQcow2State *s = bs->opaque;
 int i, j = 0, l2_index, ret;
@@ -1014,7 +1015,8 @@ out:
  *   information on cluster allocation may be invalid now. The caller
  *   must start over anyway, so consider *cur_bytes undefined.
  */
-static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
+static int coroutine_fn
+handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
 uint64_t *cur_bytes, QCowL2Meta **m)
 {
 BDRVQcow2State *s = bs->opaque;
@@ -1413,7 +1415,8 @@ fail:
  *
  * Return 0 on success and -errno in error cases
  */
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *host_offset,
QCowL2Meta **m)
 {
diff --git a/block/qcow2.c b/block/qcow2.c
index 2f94f0326e..6ecf1489dc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2079,7 +2079,8 @@ static int qcow2_change_backing_file(BlockDriverState *bs,
 return qcow2_update_header(bs);
 }
 
-static int preallocate(BlockDriverState *bs)
+static int coroutine_fn
+preallocate(BlockDriverState *bs)
 {
 uint64_t bytes;
 uint64_t offset;
@@ -2140,7 +2141,8 @@ static int preallocate(BlockDriverState *bs)
 return 0;
 }
 
-static int qcow2_create2(const char *filename, int64_t total_size,
+static int coroutine_fn
+qcow2_create2(const char *filename, int64_t total_size,
  const char *backing_file, const char *backing_format,
  int flags, size_t cluster_size, PreallocMode prealloc,
  QemuOpts *opts, int version, int refcount_order,
@@ -2390,7 +2392,8 @@ out:
 return ret;
 }
 
-static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 char *backing_file = NULL;
 char *backing_fmt = NULL;
@@ -3011,7 +3014,8 @@ static void dump_refcounts(BlockDriverState *bs)
 }
 #endif
 
-static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
   int64_t pos)
 {
 BDRVQcow2State *s = bs->opaque;
@@ -3021,7 +3025,8 @@ 

[Qemu-block] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 include/block/block_int.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 15fa602150..93eb2a9528 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -133,15 +133,15 @@ struct BlockDriver {
 void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
 
 /* aio */
-BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_readv)(BlockDriverState *bs,
 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
 BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_writev)(BlockDriverState *bs,
 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
 BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_flush)(BlockDriverState *bs,
 BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_pdiscard)(BlockDriverState *bs,
 int64_t offset, int bytes,
 BlockCompletionFunc *cb, void *opaque);
 
@@ -247,7 +247,7 @@ struct BlockDriver {
 void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
 /* to control generic scsi devices */
-BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
 unsigned long int req, void *buf,
 BlockCompletionFunc *cb, void *opaque);
 int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs,
-- 
2.13.1.395.gf7b71de06

[Qemu-block] [PATCH 02/35] WIP: coroutine: manually tag the fast-paths

[Marc-André Lureau]

Some functions are both regular and coroutine. They may call coroutine
functions in some cases, if it is known to be running in a coroutine.

Signed-off-by: Marc-André Lureau 
---
 block.c |  2 ++
 block/block-backend.c   |  2 ++
 block/io.c  | 16 +++-
 block/sheepdog.c|  2 ++
 block/throttle-groups.c | 10 --
 migration/rdma.c|  2 ++
 6 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 694396281b..b08c006da4 100644
--- a/block.c
+++ b/block.c
@@ -443,7 +443,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
 
 if (qemu_in_coroutine()) {
 /* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
 bdrv_create_co_entry();
+co_role_release(_coroutine_fn);
 } else {
 co = qemu_coroutine_create(bdrv_create_co_entry, );
 qemu_coroutine_enter(co);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0df3457a09..56fc0a4d1e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1072,7 +1072,9 @@ static int blk_prw(BlockBackend *blk, int64_t offset, 
uint8_t *buf,
 
 if (qemu_in_coroutine()) {
 /* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
 co_entry();
+co_role_release(_coroutine_fn);
 } else {
 Coroutine *co = qemu_coroutine_create(co_entry, );
 bdrv_coroutine_enter(blk_bs(blk), co);
diff --git a/block/io.c b/block/io.c
index 2de7c77983..14b88c8609 100644
--- a/block/io.c
+++ b/block/io.c
@@ -229,7 +229,9 @@ static void coroutine_fn 
bdrv_co_yield_to_drain(BlockDriverState *bs)
 void bdrv_drained_begin(BlockDriverState *bs)
 {
 if (qemu_in_coroutine()) {
+co_role_acquire(_coroutine_fn);
 bdrv_co_yield_to_drain(bs);
+co_role_release(_coroutine_fn);
 return;
 }
 
@@ -616,7 +618,9 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
 
 if (qemu_in_coroutine()) {
 /* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
 bdrv_rw_co_entry();
+co_role_release(_coroutine_fn);
 } else {
 co = qemu_coroutine_create(bdrv_rw_co_entry, );
 bdrv_coroutine_enter(child->bs, co);
@@ -1901,7 +1905,9 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
 
 if (qemu_in_coroutine()) {
 /* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
 bdrv_get_block_status_above_co_entry();
+co_role_release(_coroutine_fn);
 } else {
 co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry,
);
@@ -2027,7 +2033,11 @@ bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector 
*qiov, int64_t pos,
 bool is_read)
 {
 if (qemu_in_coroutine()) {
-return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+int ret;
+co_role_acquire(_coroutine_fn);
+ret = bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+co_role_release(_coroutine_fn);
+return ret;
 } else {
 BdrvVmstateCo data = {
 .bs = bs,
@@ -2259,7 +2269,9 @@ int bdrv_flush(BlockDriverState *bs)
 
 if (qemu_in_coroutine()) {
 /* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
 bdrv_flush_co_entry(_co);
+co_role_release(_coroutine_fn);
 } else {
 co = qemu_coroutine_create(bdrv_flush_co_entry, _co);
 bdrv_coroutine_enter(bs, co);
@@ -2406,7 +2418,9 @@ int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, 
int bytes)
 
 if (qemu_in_coroutine()) {
 /* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
 bdrv_pdiscard_co_entry();
+co_role_release(_coroutine_fn);
 } else {
 co = qemu_coroutine_create(bdrv_pdiscard_co_entry, );
 bdrv_coroutine_enter(bs, co);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 08d7b11e9d..83bc43dde4 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -726,7 +726,9 @@ static int do_req(int sockfd, BlockDriverState *bs, 
SheepdogReq *hdr,
 };
 
 if (qemu_in_coroutine()) {
+co_role_acquire(_coroutine_fn);
 do_co_req();
+co_role_release(_coroutine_fn);
 } else {
 co = qemu_coroutine_create(do_co_req, );
 if (bs) {
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index da2b490c38..8778f78965 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -304,9 +304,15 @@ static void schedule_next_request(BlockBackend *blk, bool 
is_write)
 
 /* If it doesn't have to wait, queue it for immediate execution */
 if (!must_wait) {
+bool handled = false;
+
+if (qemu_in_coroutine()) {
+co_role_acquire(_coroutine_fn);
+handled = 

[Qemu-block] [PATCH 07/35] blockjob: mark coroutine_fn

[Marc-André Lureau]

/home/elmarco/src/qemu/blockjob.c:820:9: error: calling function 
'qemu_coroutine_yield' requires holding role '_coroutine_fn' exclusively 
[-Werror,-Wthread-safety-analysis]
qemu_coroutine_yield();
^
/home/elmarco/src/qemu/blockjob.c:824:5: error: calling function 
'block_job_pause_point' requires holding role '_coroutine_fn' exclusively 
[-Werror,-Wthread-safety-analysis]
block_job_pause_point(job);
^
Signed-off-by: Marc-André Lureau 
---
 include/block/blockjob_int.h | 4 ++--
 blockjob.c   | 6 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index f13ad05c0d..a3bc01fd51 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -145,7 +145,7 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
  * Put the job to sleep (assuming that it wasn't canceled) for @ns
  * nanoseconds.  Canceling the job will interrupt the wait immediately.
  */
-void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
+void coroutine_fn block_job_sleep_ns(BlockJob *job, QEMUClockType type, 
int64_t ns);
 
 /**
  * block_job_yield:
@@ -153,7 +153,7 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, 
int64_t ns);
  *
  * Yield the block job coroutine.
  */
-void block_job_yield(BlockJob *job);
+void coroutine_fn block_job_yield(BlockJob *job);
 
 /**
  * block_job_pause_all:
diff --git a/blockjob.c b/blockjob.c
index 70a78188b7..96424323c1 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -788,7 +788,8 @@ bool block_job_is_cancelled(BlockJob *job)
 return job->cancelled;
 }
 
-void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
+void coroutine_fn
+block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
 {
 assert(job->busy);
 
@@ -806,7 +807,8 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, 
int64_t ns)
 block_job_pause_point(job);
 }
 
-void block_job_yield(BlockJob *job)
+void coroutine_fn
+block_job_yield(BlockJob *job)
 {
 assert(job->busy);
 
-- 
2.13.1.395.gf7b71de06

Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it

[Kashyap Chamarthy]

On Wed, Jun 28, 2017 at 03:33:49PM -0500, Eric Blake wrote:
> On 06/28/2017 03:15 PM, Alberto Garcia wrote:
> > On Wed 28 Jun 2017 04:58:00 PM CEST, Kashyap Chamarthy wrote:
> >> This patch documents (including their QMP invocations) all the four
> >> major kinds of live block operations:
> >>
> >>   - `block-stream`
> >>   - `block-commit`
> >>   - `drive-mirror` (& `blockdev-mirror`)
> >>   - `drive-backup` (& `blockdev-backup`)
> > 
> > This is excellent work, thanks for doing this!

Thanks!

> > I haven't had the time to review the complete document yet, but here are
> > my comments from the first half:

[I'm responding out of order -- as I first replied to your other email,
which gave feedback about the sceond half.]

> >> +Disk image backing chain notation
> >> +-
> >   [...]
> >> +.. important::
> >> +The base disk image can be raw format; however, all the overlay
> >> +files must be of QCOW2 format.
> > 
> > This is not quite like that: overlay files must be in a format that
> > supports backing files. QCOW2 is the most common one, but there are
> > others (qed). Grep for 'supports_backing' in the source code.
> 
> At the same time, other image formats are not as frequently tested, or
> may be read-only.  Maybe a compromise of "The overlay files can
> generally be any format that supports a backing file, although qcow2 is
> the preferred format and the one used in this document".

Yes, I'll use Eric's wording [thanks] here, that makes the intent
clearer.

> >> +(1) ``block-stream``: Live copy of data from backing files into overlay
> >> +files (with the optional goal of removing the backing file from the
> >> +chain).
> > 
> > optional? The backing file is removed from the chain as soon as the
> > operation finishes, although the image file is not removed from the
> > disk. Maybe you meant that?
>
> Hmm, you're right. In this case, qemu ALWAYS rewrites the backing chain
> to get rid of the (now-streamed) backing image.

Correct.  I will clarify the wording here.

> >> +(2) ``block-commit``: Live merge of data from overlay files into backing
> >> +files (with the optional goal of removing the overlay file from the
> >> +chain).  Since QEMU 2.0, this includes "active ``block-commit``"
> >> +(i.e.  merge the current active layer into the base image).
> > 
> > Same question about the 'optional' here.
> 
> Here, optional is a bit more correct. With non-active (intermediate)
> commit, qemu ALWAYS rewrites the backing chain to be shorter; but with
> live commit, you can chose whether to pivot to the now-shorter chain
> (job-complete) or whether to keep the active image intact (starting to
> collect a new delta from the point-in-time of the just-completed commit,
> by job-cancel).

Right.  I'll workout a way to mention this, too.  [Me will not wonder
whether it will confuse the reader about mentioning it so early -- as
the said reader will be using these primitives to make higher level
tools, and they can easily figure out the semantics.]
 
> >> +writing to it.  (The rule of thumb is: live QEMU will always be pointing
> >> +to the right-most image in a disk image chain.)
> > 
> > I think it's 'rightmost', without the hyphen.
> 
> Sadly, I think this is one case where both spellings work to a native
> reader, and where I don't know of a specific style-guide preference.  I
> probably would have written with the hyphen.

Heh, indeed.  Peter Maydell has said Oxford English Dictionary has
mentions for both variants.  But Alberto is correct that the non-hyphen
variant, "rightmost", is much more widely used.

> >> +(3) Intermediate streaming (available since QEMU 2.8): Starting afresh
> >> +with the original example disk image chain, with a total of four
> >> +images, it is possible to copy contents from image [B] into image
> >> +[C].  Once the copy is finished, image [B] can now be (optionally)
> >> +discarded; and the backing file pointer of image [C] will be
> >> +adjusted to point to [A].
> > 
> > The 'optional' usage again. [B] will be removed from the chain and can
> > be (optionally) removed from the disk, but that you have to do yourself,
> > QEMU won't do that.
> 
> Indeed, we may need to be specifically clear of the cases where qemu
> shortens the chain, but where disk images that are no longer used by the
> chain (whether they are still viable [as in stream], or invalidated [as
> in commit crossing more than one element of the chain]) are still left
> on the disk for the user to discard separately from qemu.

Yes, I'll keep this in mind for v5.

> >> +The ``block-commit`` command lets you to live merge data from overlay
> >> +images into backing file(s).
> > 
> > I don't think "lets you to live merge data" is correct English.
> 
> Probably better as "lets you merge live data from overlay..."

Yes.  Will fix.

[...]

> >> +(1) Commit content from only image [B] into image [A].  The resulting
> >> +chain is the 

Re: [Qemu-block] [PATCH v3 05/20] commit: Switch commit_populate() to byte-based

[Kevin Wolf]

Am 27.06.2017 um 21:24 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Start by converting an
> internal function (no semantic change).
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 

Reviewed-by: Kevin Wolf 

Re: [Qemu-block] [PATCH v3 06/20] commit: Switch commit_run() to byte-based

[Kevin Wolf]

Am 27.06.2017 um 21:24 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Change the internal
> loop iteration of committing to track by bytes instead of sectors
> (although we are still guaranteed that we iterate by steps that
> are sector-aligned).
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 

Reviewed-by: Kevin Wolf 

Re: [Qemu-block] [PATCH v3 02/20] trace: Show blockjob actions via bytes, not sectors

[Kevin Wolf]

Am 27.06.2017 um 21:24 hat Eric Blake geschrieben:
> Upcoming patches are going to switch to byte-based interfaces
> instead of sector-based.  Even worse, trace_backup_do_cow_enter()
> had a weird mix of cluster and sector indices.
> 
> The trace interface is low enough that there are no stability
> guarantees, and therefore nothing wrong with changing our units,
> even in cases like trace_backup_do_cow_skip() where we are not
> changing the trace output.  So make the tracing uniformly use
> bytes.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 

Reviewed-by: Kevin Wolf 

Re: [Qemu-block] [PATCH v3 03/20] stream: Switch stream_populate() to byte-based

[Kevin Wolf]

Am 27.06.2017 um 21:24 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Start by converting an
> internal function (no semantic change).
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 

Reviewed-by: Kevin Wolf 

Re: [Qemu-block] [PATCH v3 01/20] blockjob: Track job ratelimits via bytes, not sectors

[Kevin Wolf]

Am 27.06.2017 um 21:24 hat Eric Blake geschrieben:
> The user interface specifies job rate limits in bytes/second.
> It's pointless to have our internal representation track things
> in sectors/second, particularly since we want to move away from
> sector-based interfaces.
> 
> Fix up a doc typo found while verifying that the ratelimit
> code handles the scaling difference.
> 
> Repetition of expressions like 'n * BDRV_SECTOR_SIZE' will be
> cleaned up later when functions are converted to iterate over
> images by bytes rather than by sectors.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 

Reviewed-by: Kevin Wolf 

Re: [Qemu-block] [PATCH v3 04/20] stream: Switch stream_run() to byte-based

[Kevin Wolf]

Am 27.06.2017 um 21:24 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Change the internal
> loop iteration of streaming to track by bytes instead of sectors
> (although we are still guaranteed that we iterate by steps that
> are sector-aligned).
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 
> 
> ---
> v2: no change
> ---
>  block/stream.c | 24 ++--
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/block/stream.c b/block/stream.c
> index 746d525..2f9618b 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -108,12 +108,11 @@ static void coroutine_fn stream_run(void *opaque)
>  BlockBackend *blk = s->common.blk;
>  BlockDriverState *bs = blk_bs(blk);
>  BlockDriverState *base = s->base;
> -int64_t sector_num = 0;
> -int64_t end = -1;

Here, end was initialised to -1. This made a differnce for early 'goto
out' paths because otherwise data->reached_end would incorrectly be true
in stream_complete.

Because we also check data->ret, I think the only case where it actually
makes a difference is for the !bs->backing case: This used to result in
data->reached_end == false, but now it ends up as true. This is because
s->common.len hasn't been set yet, so it is still 0.

Kevin

Re: [Qemu-block] [PATCH v2] blockdev: Print a warning for legacy drive options that belong to -device

[Kevin Wolf]

Am 12.05.2017 um 12:33 hat Thomas Huth geschrieben:
> We likely do not want to carry these legacy -drive options along forever.
> Let's emit a deprecation warning for the -drive options that have a
> replacement with the -device option, so that the (hopefully few) remaining
> users are aware of this and can adapt their scripts / behaviour accordingly.
> 
> Signed-off-by: Thomas Huth 
> ---
>  v2:
>  - Check for !qtest_enabled() since tests/hd-geo-test still uses these
>  - Added "addr" to the list, too
>  - Also mark the options as deprecated in the documentation

Thanks, added the missing deprecation note for 'serial' in the
documentation and applied to the block branch.

Kevin

Re: [Qemu-block] [PATCH v3 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Cornelia Huck]

On Tue,  4 Jul 2017 10:32:31 +0200
QingFeng Hao  wrote:

> This patch is based on a similar patch from Stefan Hajnoczi -
> commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)
> 
> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
> 
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.

It might be good to add a sentence that we don't have an equivalent to
"memory: emulate ioeventfd" for ccw yet, but that this doesn't hurt.

> 
> I have tested that virtio-scsi-ccw works under tcg both with and without
> iothread.
> 
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
> 
> Signed-off-by: QingFeng Hao 
> ---
>  hw/s390x/virtio-ccw.c | 2 +-
>  target/s390x/cpu.h| 6 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice 
> *dev, Error **errp)
>  sch->cssid, sch->ssid, sch->schid, sch->devno,
>  ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>  
> -if (!kvm_eventfds_enabled()) {
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
>  dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>  }
>  
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 9faca04b52..bdb9bdbc9d 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -1264,7 +1264,11 @@ static inline int 
> s390_assign_subch_ioeventfd(EventNotifier *notifier,
>uint32_t sch_id, int vq,
>bool assign)
>  {
> -return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +if (kvm_enabled()) {
> +return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +} else {
> +return 0;
> +}
>  }
>  
>  static inline void s390_crypto_reset(void)

Reviewed-by: Cornelia Huck 

Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Christian Borntraeger]

On 07/04/2017 03:23 PM, QingFeng Hao wrote:
> This patch is based on a similar patch from Stefan Hajnoczi -
> commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")
> 
> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
> 
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.
> Currently we don't have an equivalent to "memory: emulate ioeventfd"
> for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
> skipping iothread arguments.
> 
> I have tested that virtio-scsi-ccw works under tcg both with and without
> iothread.
> 
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
> 
> Signed-off-by: QingFeng Hao 
> Reviewed-by: Cornelia Huck 

I will take it via the s390-next tree.

thanks applied.

> ---
>  hw/s390x/virtio-ccw.c | 2 +-
>  target/s390x/cpu.h| 6 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice 
> *dev, Error **errp)
>  sch->cssid, sch->ssid, sch->schid, sch->devno,
>  ccw_dev->devno.valid ? "user-configured" : "auto-configured");
> 
> -if (!kvm_eventfds_enabled()) {
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
>  dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>  }
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 9faca04b52..bdb9bdbc9d 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -1264,7 +1264,11 @@ static inline int 
> s390_assign_subch_ioeventfd(EventNotifier *notifier,
>uint32_t sch_id, int vq,
>bool assign)
>  {
> -return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +if (kvm_enabled()) {
> +return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +} else {
> +return 0;
> +}
>  }
> 
>  static inline void s390_crypto_reset(void)
> 

Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img: delete the -e and -6 options from the 'create' command

[Kashyap Chamarthy]

On Tue, Jul 04, 2017 at 11:34:27AM +0100, Daniel P. Berrange wrote:
> The '-e' and '-6' options to the 'create' command were "deprecated"
> in favour of the more generic '-o' option many years ago:
> 
>   commit eec77d9e712bd4157a4e1c0b5a9249d168add738
>   Author: Jes Sorensen 
>   Date:   Tue Dec 7 17:44:34 2010 +0100
> 
> qemu-img: Deprecate obsolete -6 and -e options
> 
> Except this was never actually a deprecation, which would imply giving
> the user a warning while the functionality continues to work for a
> number of releases before eventual removal. Instead the options were
> immediately turned into an error + exit. Given that the functionality
> is already broken, there's no point in keeping these psuedo-deprecation
> messages around any longer.

Sounds and looks good.

> Signed-off-by: Daniel P. Berrange 
> ---
>  qemu-img.c | 10 +-
>  1 file changed, 1 insertion(+), 9 deletions(-)

FWIW:

Reviewed-by: Kashyap Chamarthy 

> diff --git a/qemu-img.c b/qemu-img.c
> index 91ad6be..a65239f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -464,7 +464,7 @@ static int img_create(int argc, char **argv)
>  {"object", required_argument, 0, OPTION_OBJECT},
>  {0, 0, 0, 0}
>  };
> -c = getopt_long(argc, argv, ":F:b:f:he6o:q",
> +c = getopt_long(argc, argv, ":F:b:f:ho:q",
>  long_options, NULL);
>  if (c == -1) {
>  break;
> @@ -488,14 +488,6 @@ static int img_create(int argc, char **argv)
>  case 'f':
>  fmt = optarg;
>  break;
> -case 'e':
> -error_report("option -e is deprecated, please use \'-o "
> -  "encryption\' instead!");
> -goto fail;
> -case '6':
> -error_report("option -6 is deprecated, please use \'-o "
> -  "compat6\' instead!");
> -goto fail;
>  case 'o':
>  if (!is_valid_option_list(optarg)) {
>  error_report("Invalid option list: %s", optarg);
> -- 
> 2.9.4
> 
> 

-- 
/kashyap

[Qemu-block] [PATCH v4 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
Currently we don't have an equivalent to "memory: emulate ioeventfd"
for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
skipping iothread arguments.

I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao 
Reviewed-by: Cornelia Huck 
---
 hw/s390x/virtio-ccw.c | 2 +-
 target/s390x/cpu.h| 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, 
Error **errp)
 sch->cssid, sch->ssid, sch->schid, sch->devno,
 ccw_dev->devno.valid ? "user-configured" : "auto-configured");
 
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
 dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
 }
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9faca04b52..bdb9bdbc9d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int 
s390_assign_subch_ioeventfd(EventNotifier *notifier,
   uint32_t sch_id, int vq,
   bool assign)
 {
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
 }
 
 static inline void s390_crypto_reset(void)
-- 
2.11.2

[Qemu-block] [PATCH v4 0/1] virtio-scsi-ccw: fix iotest 068 for s390x

[QingFeng Hao]

This commit fixes iotest 068 for s390x as s390x uses virtio-scsi-ccw.
It's based on commit c324fd0a39c by Stefan Hajnoczi. 
Thanks!

Change history:
v4:
Got Cornelia Huck's Reviewed-by and take the comment to change the
commit message.

v3:
Take Christian Borntraeger and Cornelia Huck's comment to check
if kvm is enabled in s390_assign_subch_ioeventfd instead of
kvm_s390_assign_subch_ioeventfd to as the former is a general one.

v2:
Remove Stefan from sign-off list and change the patch's commit message 
according to Christian Borntraeger's comment.

QingFeng Hao (1):
  virtio-scsi-ccw: use ioeventfd even when KVM is disabled

 hw/s390x/virtio-ccw.c | 2 +-
 target/s390x/cpu.h| 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.11.2

Re: [Qemu-block] [PATCH v2] qemu-img: drop -e and -6 options from the 'create' & 'convert' commands

[Kevin Wolf]

Am 04.07.2017 um 14:30 hat Daniel P. Berrange geschrieben:
> The '-e' and '-6' options to the 'create' & 'convert' commands were
> "deprecated" in favour of the more generic '-o' option many years ago:
> 
>   commit eec77d9e712bd4157a4e1c0b5a9249d168add738
>   Author: Jes Sorensen 
>   Date:   Tue Dec 7 17:44:34 2010 +0100
> 
> qemu-img: Deprecate obsolete -6 and -e options
> 
> Except this was never actually a deprecation, which would imply giving
> the user a warning while the functionality continues to work for a
> number of releases before eventual removal. Instead the options were
> immediately turned into an error + exit. Given that the functionality
> is already broken, there's no point in keeping these psuedo-deprecation
> messages around any longer.
> 
> Signed-off-by: Daniel P. Berrange 

Thanks, I updated the patch in my queue with this version.

Kevin

[Qemu-block] [PATCH v2] qemu-img: drop -e and -6 options from the 'create' & 'convert' commands

[Daniel P. Berrange]

The '-e' and '-6' options to the 'create' & 'convert' commands were
"deprecated" in favour of the more generic '-o' option many years ago:

  commit eec77d9e712bd4157a4e1c0b5a9249d168add738
  Author: Jes Sorensen 
  Date:   Tue Dec 7 17:44:34 2010 +0100

qemu-img: Deprecate obsolete -6 and -e options

Except this was never actually a deprecation, which would imply giving
the user a warning while the functionality continues to work for a
number of releases before eventual removal. Instead the options were
immediately turned into an error + exit. Given that the functionality
is already broken, there's no point in keeping these psuedo-deprecation
messages around any longer.

Signed-off-by: Daniel P. Berrange 
---
 qemu-img.c | 20 ++--
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 91ad6be..c5f00db 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -464,7 +464,7 @@ static int img_create(int argc, char **argv)
 {"object", required_argument, 0, OPTION_OBJECT},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":F:b:f:he6o:q",
+c = getopt_long(argc, argv, ":F:b:f:ho:q",
 long_options, NULL);
 if (c == -1) {
 break;
@@ -488,14 +488,6 @@ static int img_create(int argc, char **argv)
 case 'f':
 fmt = optarg;
 break;
-case 'e':
-error_report("option -e is deprecated, please use \'-o "
-  "encryption\' instead!");
-goto fail;
-case '6':
-error_report("option -6 is deprecated, please use \'-o "
-  "compat6\' instead!");
-goto fail;
 case 'o':
 if (!is_valid_option_list(optarg)) {
 error_report("Invalid option list: %s", optarg);
@@ -1985,7 +1977,7 @@ static int img_convert(int argc, char **argv)
 {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:WU",
+c = getopt_long(argc, argv, ":hf:O:B:co:s:l:S:pt:T:qnm:WU",
 long_options, NULL);
 if (c == -1) {
 break;
@@ -2012,14 +2004,6 @@ static int img_convert(int argc, char **argv)
 case 'c':
 s.compressed = true;
 break;
-case 'e':
-error_report("option -e is deprecated, please use \'-o "
-  "encryption\' instead!");
-goto fail_getopt;
-case '6':
-error_report("option -6 is deprecated, please use \'-o "
-  "compat6\' instead!");
-goto fail_getopt;
 case 'o':
 if (!is_valid_option_list(optarg)) {
 error_report("Invalid option list: %s", optarg);
-- 
2.9.4

Re: [Qemu-block] [PATCH] qemu-img: delete the -e and -6 options from the 'create' command

[Daniel P. Berrange]

On Tue, Jul 04, 2017 at 02:16:06PM +0200, Kevin Wolf wrote:
> Am 04.07.2017 um 12:34 hat Daniel P. Berrange geschrieben:
> > The '-e' and '-6' options to the 'create' command were "deprecated"
> > in favour of the more generic '-o' option many years ago:
> > 
> >   commit eec77d9e712bd4157a4e1c0b5a9249d168add738
> >   Author: Jes Sorensen 
> >   Date:   Tue Dec 7 17:44:34 2010 +0100
> > 
> > qemu-img: Deprecate obsolete -6 and -e options
> > 
> > Except this was never actually a deprecation, which would imply giving
> > the user a warning while the functionality continues to work for a
> > number of releases before eventual removal. Instead the options were
> > immediately turned into an error + exit. Given that the functionality
> > is already broken, there's no point in keeping these psuedo-deprecation
> > messages around any longer.
> > 
> > Signed-off-by: Daniel P. Berrange 
> 
> Thanks, applied to the block branch.
> 
> Do you want to send another patch to do the same in qemu-img convert?

Opps, I wasn't paying attention & missed the convert command. I'll just
send a v2 of this that covers both.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-block] [PATCH] qemu-img: delete the -e and -6 options from the 'create' command

[Kevin Wolf]

Am 04.07.2017 um 12:34 hat Daniel P. Berrange geschrieben:
> The '-e' and '-6' options to the 'create' command were "deprecated"
> in favour of the more generic '-o' option many years ago:
> 
>   commit eec77d9e712bd4157a4e1c0b5a9249d168add738
>   Author: Jes Sorensen 
>   Date:   Tue Dec 7 17:44:34 2010 +0100
> 
> qemu-img: Deprecate obsolete -6 and -e options
> 
> Except this was never actually a deprecation, which would imply giving
> the user a warning while the functionality continues to work for a
> number of releases before eventual removal. Instead the options were
> immediately turned into an error + exit. Given that the functionality
> is already broken, there's no point in keeping these psuedo-deprecation
> messages around any longer.
> 
> Signed-off-by: Daniel P. Berrange 

Thanks, applied to the block branch.

Do you want to send another patch to do the same in qemu-img convert?

Kevin

Re: [Qemu-block] [RFC PATCH v2 16/15] block: Add .bdrv_co_block_status() callback

[Fam Zheng]

On Mon, 07/03 17:20, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Now that the block layer exposes byte-based allocation,
> it's time to tackle the drivers.  Add a new callback that operates
> on as small as byte boundaries. Subsequent patches will then update
> individual drivers, then finally remove .bdrv_co_get_block_status().
> The old code now uses a goto in order to minimize churn at that later
> removal.
> 
> The new code also passes through the 'allocation' hint, which will
> allow subsequent patches to further optimize callers that only care
> about how much of the image is allocated, rather than which offsets
> the allocation actually maps to.
> 
> Note that most drivers give sector-aligned answers, except at
> end-of-file, even when request_alignment is smaller than a sector.
> However, bdrv_getlength() is sector-aligned (even though it gives a
> byte answer), often by exceeding the actual file size.  If we were to
> give back strict results, at least file-posix.c would report a
> transition from DATA to HOLE at the end of a file even in the middle
> of a sector, which can throw off callers; so we intentionally lie and
> state that any partial sector at the end of a file has the same
> status for the entire sector.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v2: improve alignment handling, add additional 'allocation' argument
> for future optimization potential, ensure all iotests still pass
> 
> Sending as an RFC as part of my third series; this patch is technically
> the start of my fourth series, but given the rebase churn I've had
> elsewhere, it can't hurt to get the interface looked at in case it
> needs tweaking
> ---
>  include/block/block_int.h |  9 -
>  block/io.c| 27 ---
>  2 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 5f6ba5d..45ff534 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -172,13 +172,20 @@ struct BlockDriver {
>   * bdrv_is_allocated[_above].  The driver should answer only
>   * according to the current layer, and should not set
>   * BDRV_BLOCK_ALLOCATED, but may set BDRV_BLOCK_RAW.  See block.h
> - * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.  The block
> + * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.  As a hint,
> + * the flag allocation is true if the caller cares more about
> + * learning how much of the image is allocated, without regards to
> + * a breakdown by offset (a driver may either ignore the hint, or
> + * avoid _OFFSET_VALID to provide a larger *pnum).  The block
>   * layer guarantees input aligned to request_alignment, as well as
>   * non-NULL pnum and file.
>   */
>  int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
>  int64_t sector_num, int nb_sectors, int *pnum,
>  BlockDriverState **file);
> +int64_t coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bd,
> +bool allocation, int64_t offset, int64_t bytes, int64_t *pnum,
> +BlockDriverState **file);

Apart from my earlier question regarding the naming of "allocation", looks fine.

I haven't reviewed the implementation though.

Fam

[Qemu-block] [PATCH] qemu-img: delete the -e and -6 options from the 'create' command

[Daniel P. Berrange]

The '-e' and '-6' options to the 'create' command were "deprecated"
in favour of the more generic '-o' option many years ago:

  commit eec77d9e712bd4157a4e1c0b5a9249d168add738
  Author: Jes Sorensen 
  Date:   Tue Dec 7 17:44:34 2010 +0100

qemu-img: Deprecate obsolete -6 and -e options

Except this was never actually a deprecation, which would imply giving
the user a warning while the functionality continues to work for a
number of releases before eventual removal. Instead the options were
immediately turned into an error + exit. Given that the functionality
is already broken, there's no point in keeping these psuedo-deprecation
messages around any longer.

Signed-off-by: Daniel P. Berrange 
---
 qemu-img.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 91ad6be..a65239f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -464,7 +464,7 @@ static int img_create(int argc, char **argv)
 {"object", required_argument, 0, OPTION_OBJECT},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":F:b:f:he6o:q",
+c = getopt_long(argc, argv, ":F:b:f:ho:q",
 long_options, NULL);
 if (c == -1) {
 break;
@@ -488,14 +488,6 @@ static int img_create(int argc, char **argv)
 case 'f':
 fmt = optarg;
 break;
-case 'e':
-error_report("option -e is deprecated, please use \'-o "
-  "encryption\' instead!");
-goto fail;
-case '6':
-error_report("option -6 is deprecated, please use \'-o "
-  "compat6\' instead!");
-goto fail;
 case 'o':
 if (!is_valid_option_list(optarg)) {
 error_report("Invalid option list: %s", optarg);
-- 
2.9.4

Re: [Qemu-block] [PATCH v3 4/4] block: Add default implementations for bdrv_co_get_block_status()

[Kevin Wolf]

Am 04.07.2017 um 10:40 hat Manos Pitsidianakis geschrieben:
> bdrv_co_get_block_status_from_file() and
> bdrv_co_get_block_status_from_backing() set *file to bs->file and
> bs->backing respectively, so that bdrv_co_get_block_status() can recurse
> to them. Future block drivers won't have to duplicate code to implement
> this.
> 
> Reviewed-by: Fam Zheng 
> Reviewed-by: Eric Blake 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Manos Pitsidianakis 

Reviewed-by: Kevin Wolf 

Re: [Qemu-block] [PATCH v3 3/4] block: Remove bdrv_truncate callback in blkdebug

[Kevin Wolf]

Am 04.07.2017 um 10:40 hat Manos Pitsidianakis geschrieben:
> Now that bdrv_truncate is passed to bs->file by default, remove the
> callback from block/blkdebug.c
> 
> Signed-off-by: Manos Pitsidianakis 

We'll need to set .is_filter = true for blkdebug now. I haven't yet
looked into the existing implications that this has, but intuitively
this should be okay.

When you do so, please explain in the commit message the implications
that this has.

Kevin

Re: [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it

[Kashyap Chamarthy]

On Thu, Jun 29, 2017 at 02:28:55PM +0200, Alberto Garcia wrote:
> On Wed 28 Jun 2017 04:58:00 PM CEST, Kashyap Chamarthy wrote:
> > +Once a 'mirror' job has started, there are two possible actions when a
> > +``drive-mirror`` job is active:
> > +
> > +1. Issuing the command ``block-job-cancel``: will, after completing
> > +   synchronization of the content from the disk image chain to the
> > +   target image, [E] -- create a point-in-time (which is at the time of
> > +   *triggering* the cancel command) copy, contained in image [E], of the
> > +   backing file.
> 
> A point-in-time [...] copy [...] of the backing file ? That would be a
> copy of the whole chain or the topmost image (depending on the sync
> mode).

Yep you're right -- it indeed copies the whole chain.  I'll fix the
wording.

Kevin also made a good point on IRC (#qemu, OFTC): "By the way, are you
sure that block-job-cancel for mirror creates a point-in-time copy when
the cancel command is issued? Shouldn't it really be when the cancel
event is emitted?"

So, I'll spell out the mention of the event.

> > +The ``"sync": "full"``, from the above, means: copy the *entire* chain
> > +to the destination.
> 
> I think it's in general a good idea to describe the different sync modes
> before doing this, because they're not completely obvious when you first
> try to use these commands.
> 
> > +Notes on ``blockdev-mirror``
> > +
>   [...]
> > +(1) Create the target image (using ``qemu-img``), say, backup.qcow2
> 
> If we're doing a mirror here, backup.qcow2 is probably not the best
> name :-)

Yeah, I'll use the 'mirror.qcow2' unless someone has a better name :-)
I actually _did_ consider naming, but stuck with "backup" because,
`mirror` is a _kind_ of a backup, too.  And "mirror" itself is a
slightly wrong name, as Stefan Hajnoczi once correcte me elsewhere: it's
synchronizing disk contents to the target, but not "live mirroring"

> > +Live disk backup --- ``drive-backup`` and ``blockdev-backup``
> > +-
> > +
> > +The ``drive-backup`` (and its newer equivalent ``blockdev-backup``) allows
> > +you to create a point-in-time snapshot.
> > +
> > +In this case, the point-in-time is when you *start* the ``drive-backup``
> > +(or its newer equivalent ``blockdev-backup``) command.
> > +
> > +Currently, there are four different types of synchronization modes:
> > +
> > +(1) ``full`` -- Synchronize the content of entire disk image chain to
> > +the target
> > +(2) ``top`` -- Synchronize only the contents of the top-most disk image
> > +in the chain to the target
> > +(3) ``none`` -- Synchronize only the new writes from this point on
> > +(4) ``incremental`` -- Synchronize content that is described by the
> > +dirty bitmap
> 
> Here's the kind of summary of the different sync modes that I was
> talking about earlier.

I will move it to the top.  I even thought about it, but forgot to act
on it.  It makes logical sense to mention the terms on their first
occurence.

> One note about sync=none, though. If I'm not wrong drive-backup makes a
> point-in-time copy of the data at the moment when the command is issued
> (that's one difference from drive-mirror). Therefore drive-backup never
> synchronizes "new writes from this point on".
> 
> I don't think that drive-backup sync=none copies any data then, can
> anyone clarify?

Kevin clarified on IRC:

[kashyap] kwolf: When you get a moment, can you clarify Alberto's
  question on this thread:
  https://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg06670.html

[kwolf] kashyap: The documentation [I think he means QAPI schema
documentation here] isn't very clear, we should probably improve
the comment in the schema

[kwolf] But looking at the code it seems it just disables the background
copy, i.e. it backups only what is overwritten.

[kashyap] kwolf: Let's see if I get it: so it ('drive-backup sync=none')
  backups whatever the overwritten contents in the disk image
  chain to the target?

[kwolf] kashyap: Yes. Normally backup consists of two parts: Anything
that is overwritten by the guest is first copied out to the
backup, and in the background the whole image is copied start to
end. With sync=none, it's only the first part.

> The rest of the document looks good to me, thanks!

Thanks for the review.  Much appreciated.


-- 
/kashyap

Re: [Qemu-block] [PATCH v3 2/4] block: Use defaults of bdrv_* callbacks in raw

[Kevin Wolf]

Am 04.07.2017 um 10:40 hat Manos Pitsidianakis geschrieben:
> Now that passing the call to bs->file is the default for some bdrv_*
> callbacks, remove the duplicate implementations in block/raw-format.c
> 
> Reviewed-by: Eric Blake 
> Signed-off-by: Manos Pitsidianakis 

If you change patch 1 as suggested there, most of these functions will
have to stay.

The exception is .bdrv_has_zero_init, for which I suggested an
.is_filter check. However, raw isn't declared a filter currently. If we
want to declare it a filter, the definition I proposed in the other mail
doesn't work because raw can apply an offset - this why it needs its own
function for probing geometry, so with that changed definition checking
.is_filter might not be enough any more for all of the functions in
patch 1.

Maybe it turns out in the end that raw just is special enough that
having its own functions is okay.

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH v2 15/15] qemu-io: Relax 'alloc' now that block-status doesn't assert

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> Previously, the alloc command required that input parameters be
> sector-aligned and clamped to 32 bits, because the underlying
> bdrv_is_allocated used a 32-bit parameter and asserted aligned
> inputs.  But now that we have fixed block status to report a
> 64-bit bytes value, and to properly round requests on behalf of
> guests, we can pass any values, and can use qemu-io to add
> coverage that our rounding is correct regardless of the guest
> alignment constraints.
> 
> Update iotest 177 to intentionally probe block status at
> unaligned boundaries, which also required tweaking the image
> prep to leave an unallocated portion to the image under test.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [PATCH v3 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

在 2017/7/4 17:34, Cornelia Huck 写道:

On Tue,  4 Jul 2017 10:32:31 +0200
QingFeng Hao  wrote:


This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.

It might be good to add a sentence that we don't have an equivalent to
"memory: emulate ioeventfd" for ccw yet, but that this doesn't hurt.

Ok, I'll add it. thanks.



I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao 
---
  hw/s390x/virtio-ccw.c | 2 +-
  target/s390x/cpu.h| 6 +-
  2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, 
Error **errp)
  sch->cssid, sch->ssid, sch->schid, sch->devno,
  ccw_dev->devno.valid ? "user-configured" : "auto-configured");
  
-if (!kvm_eventfds_enabled()) {

+if (kvm_enabled() && !kvm_eventfds_enabled()) {
  dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
  }
  
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h

index 9faca04b52..bdb9bdbc9d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int 
s390_assign_subch_ioeventfd(EventNotifier *notifier,
uint32_t sch_id, int vq,
bool assign)
  {
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
  }
  
  static inline void s390_crypto_reset(void)

Reviewed-by: Cornelia Huck 

Thanks!




--
Regards
QingFeng Hao

Re: [Qemu-block] [PATCH v3 1/4] block: Pass bdrv_* methods to bs->file by default

[Kevin Wolf]

Am 04.07.2017 um 10:40 hat Manos Pitsidianakis geschrieben:
> The following functions fail if bs->drv does not implement them:
> 
> bdrv_probe_blocksizes
> bdrv_probe_geometry
> bdrv_truncate
> bdrv_has_zero_init
> bdrv_get_info
> bdrv_media_changed
> bdrv_eject
> bdrv_lock_medium
> bdrv_co_ioctl
> 
> Instead, the call should be passed to bs->file if it exists, to allow
> filter drivers to support those methods without implementing them.
> 
> Reviewed-by: Eric Blake 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block.c| 27 +--
>  block/io.c |  5 +
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 69439628..8a122142 100644
> --- a/block.c
> +++ b/block.c
> @@ -494,6 +494,8 @@ int bdrv_probe_blocksizes(BlockDriverState *bs, 
> BlockSizes *bsz)
>  
>  if (drv && drv->bdrv_probe_blocksizes) {
>  return drv->bdrv_probe_blocksizes(bs, bsz);
> +} else if (drv && bs->file && bs->file->bs) {

The check for bs->file->bs isn't really necessary, we never have a
BdrvChild with a NULL bs.

> +return bdrv_probe_blocksizes(bs->file->bs, bsz);
>  }
>  
>  return -ENOTSUP;

This change actually changes existing behaviour. Basically, all of your
additions also apply for qcow2 images or other image formats, and we
need to check whether they make sense there.

bdrv_probe_blocksizes() is used for the default values for the physical
and logical block sizes for block devices if no value is explicitly
specified in -device. This makes sense as it will allow the guest to
optimise its requests so that they align with the block size that is
optimal for the storage that contains the image. (I also think that we
probably should implement .bdrv_probe_blocksizes() not only for
host_device, but also for regular files to get optimal performance by
default.)

Changing the defaults can be a problem for cross-version live migration
if the default values are used. Management tools that want to do live
migration are supposed to explicitly provide all options they can, so
this should be okay.

Eric, does libvirt set the physical/logical block size explicitly? If
not, it would be good if it started to do so.

This hunk is non-trivial enough that it's probably worth a patch of its
own with a detailed commit message explaining why we want this change
and why it is safe.

> @@ -511,6 +513,8 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry 
> *geo)
>  
>  if (drv && drv->bdrv_probe_geometry) {
>  return drv->bdrv_probe_geometry(bs, geo);
> +} else if (drv && bs->file && bs->file->bs) {
> +return bdrv_probe_geometry(bs->file->bs, geo);
>  }
>  
>  return -ENOTSUP;

The probed geometry would refer to the physical image file, not to the
actually presented image. So propagating this to bs->file is wrong for
image formats.

Possibly checking .is_filter in addition would be enough.

If you decide that this is the right field, please add some note to the
comment for the .is_filter declaration that tells about the assumptions
that are made when the field is set. I'm not completely sure what they
are, but maybe something like this: If bs->file != NULL, then all data
exposed by the filter node is mapped at the same offset in bs->file.

> @@ -3406,11 +3410,15 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
> Error **errp)
>  
>  assert(child->perm & BLK_PERM_RESIZE);
>  
> +/* if bs->drv == NULL, bs is closed, so there's nothing to do here */
>  if (!drv) {
>  error_setg(errp, "No medium inserted");
>  return -ENOMEDIUM;
>  }
>  if (!drv->bdrv_truncate) {
> +if (bs->file && bs->file->bs) {
> +return bdrv_truncate(bs->file, offset, errp);
> +}
>  error_setg(errp, "Image format driver does not support resize");
>  return -ENOTSUP;
>  }

This is actively dangerous without an .is_filter check!

All image formats that don't support image resizing would instead just
truncate the image file without considering the format at all.

> @@ -3778,6 +3786,9 @@ int bdrv_has_zero_init(BlockDriverState *bs)
>  if (bs->drv->bdrv_has_zero_init) {
>  return bs->drv->bdrv_has_zero_init(bs);
>  }
> +if (bs->file && bs->file->bs) {
> +return bdrv_has_zero_init(bs->file->bs);
> +}
>  
>  /* safe default */
>  return 0;

This is wrong for image formats.

> @@ -3832,10 +3843,16 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
>  int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
>  {
>  BlockDriver *drv = bs->drv;
> -if (!drv)
> +/* if bs->drv == NULL, bs is closed, so there's nothing to do here */
> +if (!drv) {
>  return -ENOMEDIUM;
> -if (!drv->bdrv_get_info)
> +}
> +if (!drv->bdrv_get_info) {
> +if (bs->file && bs->file->bs) {
> +return bdrv_get_info(bs->file->bs, bdi);
> +}
>

Re: [Qemu-block] [PATCH v2 14/15] block: Align block status requests

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> Any device that has request_alignment greater than 512 should be
> unable to report status at a finer granularity; it may also be
> simpler for such devices to be guaranteed that the block layer
> has rounded things out to the granularity boundary (the way the
> block layer already rounds all other I/O out).  Besides, getting
> the code correct for super-sector alignment also benefits us
> for the fact that our public interface now has byte granularity,
> even though none of our drivers have byte-level callbacks.
> 
> Add an assertion in blkdebug that proves that the block layer
> never requests status of unaligned sections, similar to what it
> does on other requests (while still keeping the generic helper
> in place for when future patches add a throttle driver).  Note
> note that iotest 177 already covers this (it would fail if you

Drop one "note"?

> use just the blkdebug.c hunk without the io.c changes).
> Meanwhile, we can drop assertions in callers that no longer have
> to pass in sector-aligned addresses.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v2: new patch
> ---
>  include/block/block_int.h |  3 ++-
>  block/blkdebug.c  | 13 +++-
>  block/io.c| 53 
> ---
>  3 files changed, 50 insertions(+), 19 deletions(-)
> 
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index ffa22c7..5f6ba5d 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -173,7 +173,8 @@ struct BlockDriver {
>   * according to the current layer, and should not set
>   * BDRV_BLOCK_ALLOCATED, but may set BDRV_BLOCK_RAW.  See block.h
>   * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.  The block
> - * layer guarantees non-NULL pnum and file.
> + * layer guarantees input aligned to request_alignment, as well as
> + * non-NULL pnum and file.
>   */
>  int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
>  int64_t sector_num, int nb_sectors, int *pnum,
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index f1539db..67736b4 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -641,6 +641,17 @@ static int coroutine_fn 
> blkdebug_co_pdiscard(BlockDriverState *bs,
>  return bdrv_co_pdiscard(bs->file->bs, offset, bytes);
>  }
> 
> +static int64_t coroutine_fn blkdebug_co_get_block_status(
> +BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
> +BlockDriverState **file)
> +{
> +assert(QEMU_IS_ALIGNED(sector_num | nb_sectors,
> +   DIV_ROUND_UP(bs->bl.request_alignment,
> +BDRV_SECTOR_SIZE)));
> +return bdrv_co_get_block_status_from_file(bs, sector_num, nb_sectors,
> +  pnum, file);
> +}
> +
>  static void blkdebug_close(BlockDriverState *bs)
>  {
>  BDRVBlkdebugState *s = bs->opaque;
> @@ -915,7 +926,7 @@ static BlockDriver bdrv_blkdebug = {
>  .bdrv_co_flush_to_disk  = blkdebug_co_flush,
>  .bdrv_co_pwrite_zeroes  = blkdebug_co_pwrite_zeroes,
>  .bdrv_co_pdiscard   = blkdebug_co_pdiscard,
> -.bdrv_co_get_block_status = bdrv_co_get_block_status_from_file,
> +.bdrv_co_get_block_status = blkdebug_co_get_block_status,
> 
>  .bdrv_debug_event   = blkdebug_debug_event,
>  .bdrv_debug_breakpoint  = blkdebug_debug_breakpoint,
> diff --git a/block/io.c b/block/io.c
> index 91d3e99..5ed1ac7 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1746,7 +1746,8 @@ static int64_t coroutine_fn 
> bdrv_co_block_status(BlockDriverState *bs,
>  int64_t n; /* bytes */
>  int64_t ret, ret2;
>  BlockDriverState *local_file = NULL;
> -int count; /* sectors */
> +int64_t aligned_offset, aligned_bytes;
> +uint32_t align;
> 
>  assert(pnum);
>  total_size = bdrv_getlength(bs);
> @@ -1788,27 +1789,44 @@ static int64_t coroutine_fn 
> bdrv_co_block_status(BlockDriverState *bs,
>  }
> 
>  bdrv_inc_in_flight(bs);
> -/*
> - * TODO: Rather than require aligned offsets, we could instead
> - * round to the driver's request_alignment here, then touch up
> - * count afterwards back to the caller's expectations.
> - */
> -assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
> -ret = bs->drv->bdrv_co_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
> -bytes >> BDRV_SECTOR_BITS, 
> ,
> -_file);
> -if (ret < 0) {
> -*pnum = 0;
> -goto out;
> +
> +/* Round out to request_alignment boundaries */
> +align = MAX(bs->bl.request_alignment, BDRV_SECTOR_SIZE);
> +aligned_offset = QEMU_ALIGN_DOWN(offset, align);
> +aligned_bytes = ROUND_UP(offset + bytes, align) - aligned_offset;
> +
> +{

Not sure why using a {} block here?

> +int 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 13/15] block: Convert bdrv_get_block_status_above() to bytes

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based.  In the common case, allocation is unlikely to ever use
> values that are not naturally sector-aligned, but it is possible
> that byte-based values will let us be more precise about allocation
> at the end of an unaligned file that can do byte-based access.
> 
> Changing the name of the function from bdrv_get_block_status_above()
> to bdrv_block_status_above() ensures that the compiler enforces that
> all callers are updated.  For now, the io.c layer still assert()s
> that all callers are sector-aligned, but that can be relaxed when a
> later patch implements byte-based block status in the drivers.
> 
> For the most part this patch is just the addition of scaling at the
> callers followed by inverse scaling at bdrv_block_status().  But some
> code, particularly bdrv_block_status(), gets a lot simpler because
> it no longer has to mess with sectors.
> 
> For ease of review, bdrv_get_block_status() was tackled separately.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 12/15] block: Switch bdrv_co_get_block_status_above() to byte-based

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> type (no semantic change), and rename it to match the corresponding
> public function rename.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 10/15] block: Switch BdrvCoGetBlockStatusData to byte-based

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> type (no semantic change), and rename it to match the corresponding
> public function rename.
> 
> Signed-off-by: Eric Blake 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 11/15] block: Switch bdrv_common_block_status_above() to byte-based

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> function (no semantic change).
> 
> Signed-off-by: Eric Blake 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 09/15] block: Switch bdrv_co_get_block_status() to byte-based

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> function (no semantic change), and as with its public counterpart,
> rename to bdrv_co_block_status() to make the compiler enforce that
> we catch all uses.  For now, we assert that callers still pass
> aligned data, but ultimately, this will be the function where we
> hand off to a byte-based driver callback, and will eventually need
> to add logic to ensure we round calls according to the driver's
> request_alignment then touch up the result handed back to the
> caller, to start permitting a caller to pass unaligned offsets.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Fam Zheng 

[Qemu-block] [PATCH v3 4/4] block: Add default implementations for bdrv_co_get_block_status()

[Manos Pitsidianakis]

bdrv_co_get_block_status_from_file() and
bdrv_co_get_block_status_from_backing() set *file to bs->file and
bs->backing respectively, so that bdrv_co_get_block_status() can recurse
to them. Future block drivers won't have to duplicate code to implement
this.

Reviewed-by: Fam Zheng 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Manos Pitsidianakis 
---
 block/blkdebug.c  | 12 +---
 block/commit.c| 12 +---
 block/io.c| 26 ++
 block/mirror.c| 12 +---
 include/block/block_int.h | 18 ++
 5 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 5e118e10..5947e115 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -641,16 +641,6 @@ static int coroutine_fn 
blkdebug_co_pdiscard(BlockDriverState *bs,
 return bdrv_co_pdiscard(bs->file->bs, offset, bytes);
 }
 
-static int64_t coroutine_fn blkdebug_co_get_block_status(
-BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
-BlockDriverState **file)
-{
-*pnum = nb_sectors;
-*file = bs->file->bs;
-return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
-(sector_num << BDRV_SECTOR_BITS);
-}
-
 static void blkdebug_close(BlockDriverState *bs)
 {
 BDRVBlkdebugState *s = bs->opaque;
@@ -919,7 +909,7 @@ static BlockDriver bdrv_blkdebug = {
 .bdrv_co_flush_to_disk  = blkdebug_co_flush,
 .bdrv_co_pwrite_zeroes  = blkdebug_co_pwrite_zeroes,
 .bdrv_co_pdiscard   = blkdebug_co_pdiscard,
-.bdrv_co_get_block_status = blkdebug_co_get_block_status,
+.bdrv_co_get_block_status = bdrv_co_get_block_status_from_file,
 
 .bdrv_debug_event   = blkdebug_debug_event,
 .bdrv_debug_breakpoint  = blkdebug_debug_breakpoint,
diff --git a/block/commit.c b/block/commit.c
index 524bd549..5b04f832 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -247,16 +247,6 @@ static int coroutine_fn 
bdrv_commit_top_preadv(BlockDriverState *bs,
 return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
 
-static int64_t coroutine_fn bdrv_commit_top_get_block_status(
-BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
-BlockDriverState **file)
-{
-*pnum = nb_sectors;
-*file = bs->backing->bs;
-return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
-   (sector_num << BDRV_SECTOR_BITS);
-}
-
 static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts)
 {
 bdrv_refresh_filename(bs->backing->bs);
@@ -282,7 +272,7 @@ static void bdrv_commit_top_child_perm(BlockDriverState 
*bs, BdrvChild *c,
 static BlockDriver bdrv_commit_top = {
 .format_name= "commit_top",
 .bdrv_co_preadv = bdrv_commit_top_preadv,
-.bdrv_co_get_block_status   = bdrv_commit_top_get_block_status,
+.bdrv_co_get_block_status   = bdrv_co_get_block_status_from_backing,
 .bdrv_refresh_filename  = bdrv_commit_top_refresh_filename,
 .bdrv_close = bdrv_commit_top_close,
 .bdrv_child_perm= bdrv_commit_top_child_perm,
diff --git a/block/io.c b/block/io.c
index c22a9bf2..d28c6864 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1706,6 +1706,32 @@ typedef struct BdrvCoGetBlockStatusData {
 bool done;
 } BdrvCoGetBlockStatusData;
 
+int64_t coroutine_fn bdrv_co_get_block_status_from_file(BlockDriverState *bs,
+int64_t sector_num,
+int nb_sectors,
+int *pnum,
+BlockDriverState 
**file)
+{
+assert(bs->file && bs->file->bs);
+*pnum = nb_sectors;
+*file = bs->file->bs;
+return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
+   (sector_num << BDRV_SECTOR_BITS);
+}
+
+int64_t coroutine_fn bdrv_co_get_block_status_from_backing(BlockDriverState 
*bs,
+   int64_t sector_num,
+   int nb_sectors,
+   int *pnum,
+   BlockDriverState 
**file)
+{
+assert(bs->backing && bs->backing->bs);
+*pnum = nb_sectors;
+*file = bs->backing->bs;
+return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
+   (sector_num << BDRV_SECTOR_BITS);
+}
+
 /*
  * Returns the allocation status of the specified sectors.
  * Drivers not implementing the functionality are assumed to not support
diff --git a/block/mirror.c b/block/mirror.c
index 61a862dc..e8bf5f40 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1052,16 +1052,6 @@ static int coroutine_fn 
bdrv_mirror_top_flush(BlockDriverState *bs)
 

[Qemu-block] [PATCH v3 0/4] block: Block driver callbacks fixes

[Manos Pitsidianakis]

This series makes implementing some of the bdrv_* callbacks easier for block
drivers by passing requests to bs->file if bs->drv doesn't implement it instead
of failing, and adding default bdrv_co_get_block_status() implementations.

This is based against Kevin Wolf's block branch, commit
da4bd74d2450ab72a7c26bbabb10c6a287dd043e

v3:
  minor changes by Eric Blake's suggestion
  new patch: remove bdrv_truncate method from blkdebug

v2:
  do not pass to bs->file if bs->drv is NULL
  move bs->file check outside of bdrv_inc_in_flight() area in bdrv_co_ioctl()
  new patch: remove duplicate code from block/raw-format.c

Manos Pitsidianakis (4):
  block: Pass bdrv_* methods to bs->file by default
  block: Use defaults of bdrv_* callbacks in raw
  block: Remove bdrv_truncate callback in blkdebug
  block: Add default implementations for bdrv_co_get_block_status()

 block.c   | 27 +--
 block/blkdebug.c  | 18 +-
 block/commit.c| 12 +---
 block/io.c| 31 +++
 block/mirror.c| 12 +---
 block/raw-format.c| 30 --
 include/block/block_int.h | 18 ++
 7 files changed, 77 insertions(+), 71 deletions(-)

-- 
2.11.0

[Qemu-block] [PATCH v3 3/4] block: Remove bdrv_truncate callback in blkdebug

[Manos Pitsidianakis]

Now that bdrv_truncate is passed to bs->file by default, remove the
callback from block/blkdebug.c

Signed-off-by: Manos Pitsidianakis 
---
 block/blkdebug.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index b25856c4..5e118e10 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -821,11 +821,6 @@ static int64_t blkdebug_getlength(BlockDriverState *bs)
 return bdrv_getlength(bs->file->bs);
 }
 
-static int blkdebug_truncate(BlockDriverState *bs, int64_t offset, Error 
**errp)
-{
-return bdrv_truncate(bs->file, offset, errp);
-}
-
 static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
 {
 BDRVBlkdebugState *s = bs->opaque;
@@ -916,7 +911,6 @@ static BlockDriver bdrv_blkdebug = {
 .bdrv_child_perm= bdrv_filter_default_perms,
 
 .bdrv_getlength = blkdebug_getlength,
-.bdrv_truncate  = blkdebug_truncate,
 .bdrv_refresh_filename  = blkdebug_refresh_filename,
 .bdrv_refresh_limits= blkdebug_refresh_limits,
 
-- 
2.11.0

[Qemu-block] [PATCH v3 1/4] block: Pass bdrv_* methods to bs->file by default

[Manos Pitsidianakis]

The following functions fail if bs->drv does not implement them:

bdrv_probe_blocksizes
bdrv_probe_geometry
bdrv_truncate
bdrv_has_zero_init
bdrv_get_info
bdrv_media_changed
bdrv_eject
bdrv_lock_medium
bdrv_co_ioctl

Instead, the call should be passed to bs->file if it exists, to allow
filter drivers to support those methods without implementing them.

Reviewed-by: Eric Blake 
Signed-off-by: Manos Pitsidianakis 
---
 block.c| 27 +--
 block/io.c |  5 +
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 69439628..8a122142 100644
--- a/block.c
+++ b/block.c
@@ -494,6 +494,8 @@ int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes 
*bsz)
 
 if (drv && drv->bdrv_probe_blocksizes) {
 return drv->bdrv_probe_blocksizes(bs, bsz);
+} else if (drv && bs->file && bs->file->bs) {
+return bdrv_probe_blocksizes(bs->file->bs, bsz);
 }
 
 return -ENOTSUP;
@@ -511,6 +513,8 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry 
*geo)
 
 if (drv && drv->bdrv_probe_geometry) {
 return drv->bdrv_probe_geometry(bs, geo);
+} else if (drv && bs->file && bs->file->bs) {
+return bdrv_probe_geometry(bs->file->bs, geo);
 }
 
 return -ENOTSUP;
@@ -3406,11 +3410,15 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
Error **errp)
 
 assert(child->perm & BLK_PERM_RESIZE);
 
+/* if bs->drv == NULL, bs is closed, so there's nothing to do here */
 if (!drv) {
 error_setg(errp, "No medium inserted");
 return -ENOMEDIUM;
 }
 if (!drv->bdrv_truncate) {
+if (bs->file && bs->file->bs) {
+return bdrv_truncate(bs->file, offset, errp);
+}
 error_setg(errp, "Image format driver does not support resize");
 return -ENOTSUP;
 }
@@ -3778,6 +3786,9 @@ int bdrv_has_zero_init(BlockDriverState *bs)
 if (bs->drv->bdrv_has_zero_init) {
 return bs->drv->bdrv_has_zero_init(bs);
 }
+if (bs->file && bs->file->bs) {
+return bdrv_has_zero_init(bs->file->bs);
+}
 
 /* safe default */
 return 0;
@@ -3832,10 +3843,16 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
 BlockDriver *drv = bs->drv;
-if (!drv)
+/* if bs->drv == NULL, bs is closed, so there's nothing to do here */
+if (!drv) {
 return -ENOMEDIUM;
-if (!drv->bdrv_get_info)
+}
+if (!drv->bdrv_get_info) {
+if (bs->file && bs->file->bs) {
+return bdrv_get_info(bs->file->bs, bdi);
+}
 return -ENOTSUP;
+}
 memset(bdi, 0, sizeof(*bdi));
 return drv->bdrv_get_info(bs, bdi);
 }
@@ -4205,6 +4222,8 @@ int bdrv_media_changed(BlockDriverState *bs)
 
 if (drv && drv->bdrv_media_changed) {
 return drv->bdrv_media_changed(bs);
+} else if (drv && bs->file && bs->file->bs) {
+return bdrv_media_changed(bs->file->bs);
 }
 return -ENOTSUP;
 }
@@ -4218,6 +4237,8 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag)
 
 if (drv && drv->bdrv_eject) {
 drv->bdrv_eject(bs, eject_flag);
+} else if (drv && bs->file && bs->file->bs) {
+bdrv_eject(bs->file->bs, eject_flag);
 }
 }
 
@@ -4233,6 +4254,8 @@ void bdrv_lock_medium(BlockDriverState *bs, bool locked)
 
 if (drv && drv->bdrv_lock_medium) {
 drv->bdrv_lock_medium(bs, locked);
+} else if (drv && bs->file && bs->file->bs) {
+bdrv_lock_medium(bs->file->bs, locked);
 }
 }
 
diff --git a/block/io.c b/block/io.c
index 5c146b5a..c22a9bf2 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2425,6 +2425,11 @@ int bdrv_co_ioctl(BlockDriverState *bs, int req, void 
*buf)
 };
 BlockAIOCB *acb;
 
+if (drv && !drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl &&
+bs->file && bs->file->bs) {
+return bdrv_co_ioctl(bs->file->bs, req, buf);
+}
+
 bdrv_inc_in_flight(bs);
 if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) {
 co.ret = -ENOTSUP;
-- 
2.11.0

[Qemu-block] [PATCH v3 2/4] block: Use defaults of bdrv_* callbacks in raw

[Manos Pitsidianakis]

Now that passing the call to bs->file is the default for some bdrv_*
callbacks, remove the duplicate implementations in block/raw-format.c

Reviewed-by: Eric Blake 
Signed-off-by: Manos Pitsidianakis 
---
 block/raw-format.c | 30 --
 1 file changed, 30 deletions(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index 1ea8c2d7..ccd63411 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -312,11 +312,6 @@ static int64_t raw_getlength(BlockDriverState *bs)
 return s->size;
 }
 
-static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
-{
-return bdrv_get_info(bs->file->bs, bdi);
-}
-
 static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 {
 if (bs->probed) {
@@ -346,21 +341,6 @@ static int raw_truncate(BlockDriverState *bs, int64_t 
offset, Error **errp)
 return bdrv_truncate(bs->file, offset, errp);
 }
 
-static int raw_media_changed(BlockDriverState *bs)
-{
-return bdrv_media_changed(bs->file->bs);
-}
-
-static void raw_eject(BlockDriverState *bs, bool eject_flag)
-{
-bdrv_eject(bs->file->bs, eject_flag);
-}
-
-static void raw_lock_medium(BlockDriverState *bs, bool locked)
-{
-bdrv_lock_medium(bs->file->bs, locked);
-}
-
 static int raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
 {
 BDRVRawState *s = bs->opaque;
@@ -370,11 +350,6 @@ static int raw_co_ioctl(BlockDriverState *bs, unsigned 
long int req, void *buf)
 return bdrv_co_ioctl(bs->file->bs, req, buf);
 }
 
-static int raw_has_zero_init(BlockDriverState *bs)
-{
-return bdrv_has_zero_init(bs->file->bs);
-}
-
 static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 return bdrv_create_file(filename, opts, errp);
@@ -479,16 +454,11 @@ BlockDriver bdrv_raw = {
 .bdrv_truncate= _truncate,
 .bdrv_getlength   = _getlength,
 .has_variable_length  = true,
-.bdrv_get_info= _get_info,
 .bdrv_refresh_limits  = _refresh_limits,
 .bdrv_probe_blocksizes = _probe_blocksizes,
 .bdrv_probe_geometry  = _probe_geometry,
-.bdrv_media_changed   = _media_changed,
-.bdrv_eject   = _eject,
-.bdrv_lock_medium = _lock_medium,
 .bdrv_co_ioctl= _co_ioctl,
 .create_opts  = _create_opts,
-.bdrv_has_zero_init   = _has_zero_init
 };
 
 static void bdrv_raw_init(void)
-- 
2.11.0

Re: [Qemu-block] [Qemu-devel] [PATCH v2 08/15] block: Convert bdrv_get_block_status() to bytes

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based.  In the common case, allocation is unlikely to ever use
> values that are not naturally sector-aligned, but it is possible
> that byte-based values will let us be more precise about allocation
> at the end of an unaligned file that can do byte-based access.
> 
> Changing the name of the function from bdrv_get_block_status() to
> bdrv_block_status() ensures that the compiler enforces that all
> callers are updated.  For now, the io.c layer still assert()s that
> all callers are sector-aligned, but that can be relaxed when a later
> patch implements byte-based block status in the drivers.
> 
> Note that we have an inherent limitation in the BDRV_BLOCK_* return
> values: BDRV_BLOCK_OFFSET_VALID can only return the start of a
> sector, even if we later relax the interface to query for the status
> starting at an intermediate byte; document the obvious interpretation
> that valid offsets are always sector-relative.
> 
> Therefore, for the most part this patch is just the addition of scaling
> at the callers followed by inverse scaling at bdrv_block_status().  But
> some code, particularly bdrv_is_allocated(), gets a lot simpler because
> it no longer has to mess with sectors.
> 
> For ease of review, bdrv_get_block_status_above() will be tackled
> separately.
> 
> Signed-off-by: Eric Blake 
> 

Reviewed-by: Fam Zheng 

[Qemu-block] [PATCH v3 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.

I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao 
---
 hw/s390x/virtio-ccw.c | 2 +-
 target/s390x/cpu.h| 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, 
Error **errp)
 sch->cssid, sch->ssid, sch->schid, sch->devno,
 ccw_dev->devno.valid ? "user-configured" : "auto-configured");
 
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
 dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
 }
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9faca04b52..bdb9bdbc9d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int 
s390_assign_subch_ioeventfd(EventNotifier *notifier,
   uint32_t sch_id, int vq,
   bool assign)
 {
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
 }
 
 static inline void s390_crypto_reset(void)
-- 
2.11.2

Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

在 2017/7/4 15:06, Christian Borntraeger 写道:

On 07/04/2017 05:41 AM, QingFeng Hao wrote:


在 2017/7/3 18:20, Christian Borntraeger 写道:

On 07/03/2017 10:51 AM, QingFeng Hao wrote:

This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.

I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao 
---
   hw/s390x/virtio-ccw.c | 2 +-
   target/s390x/kvm.c| 3 +++
   2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, 
Error **errp)
   sch->cssid, sch->ssid, sch->schid, sch->devno,
   ccw_dev->devno.valid ? "user-configured" : "auto-configured");

-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
   dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
   }

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a3d00196f4..c37f9c3b9e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier 
*notifier, uint32_t sch,
   .addr = sch,
   .len = 8,
   };
+if (!kvm_enabled()) {
+return 0;
+}

thinking more about it. wouldnt it be better to do something like this instead

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 058ddad..cc47831 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1240,7 +1240,11 @@ static inline int 
s390_assign_subch_ioeventfd(EventNotifier *notifier,
 uint32_t sch_id, int vq,
 bool assign)
   {
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
   }

Thanks. It makes sense. I'll change it.

   static inline void s390_crypto_reset(void)


FWIW, it seems that we (s390) do not have a functional equivalent function as 
commit
8c56c1a592b5092d91da8d8943c1d6462a6f ("memory: emulate ioeventfd") , so we 
will
not use the iothreads.

Ok, but might s390 have skipped the iothread arguments and passed the test, so 
we could
still keep this test case?

Yes, lets just fix the test case. We can implement emulated ioeventfds later.

Fine, thanks!




--
Regards
QingFeng Hao

Re: [Qemu-block] [Qemu-devel] [PATCH v2 07/15] qemu-img: Switch get_block_status() to byte-based

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Continue by converting
> an internal function (no semantic change), and simplifying its
> caller accordingly.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/15] block: Make bdrv_round_to_clusters() signature more useful

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> In the process of converting sector-based interfaces to bytes,
> I'm finding it easier to represent a byte count as a 64-bit
> integer at the block layer (even if we are internally capped
> by SIZE_MAX or even INT_MAX for individual transactions, it's
> still nicer to not have to worry about truncation/overflow
> issues on as many variables).  Update the signature of
> bdrv_round_to_clusters() to uniformly use int64_t, matching
> the signature already chosen for bdrv_is_allocated and the
> fact that off_t is also a signed type, then adjust clients
> according to the required fallout.
> 
> Signed-off-by: Eric Blake 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 05/15] qcow2: Switch is_zero_sectors() to byte-based

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> function (no semantic change), and rename it to is_zero() in the
> process.
> 
> Signed-off-by: Eric Blake 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 06/15] block: Switch bdrv_make_zero() to byte-based

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Change the internal
> loop iteration of zeroing a device to track by bytes instead of
> sectors (although we are still guaranteed that we iterate by steps
> that are sector-aligned).
> 
> Signed-off-by: Eric Blake 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 02/15] block: Allow NULL file for bdrv_get_block_status()

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> Not all callers care about which BDS owns the mapping for a given
> range of the file.  This patch merely simplifies the callers by
> consolidating the logic in the common call point, while guaranteeing
> a non-NULL file to all the driver callbacks, for no semantic change.
> The only caller that does not care about pnum is bdrv_is_allocated,
> as invoked by vvfat; we can likewise add assertions that the rest
> of the stack does not have to worry about a NULL pnum.
> 
> Furthermore, this will also set the stage for a future cleanup: when
> a caller does not care about which BDS owns an offset, it would be
> nice to allow the driver to optimize things to not have to return
> BDRV_BLOCK_OFFSET_VALID in the first place.  In the case of fragmented
> allocation (for example, it's fairly easy to create a qcow2 image
> where consecutive guest addresses are not at consecutive host
> addresses), the current contract requires bdrv_get_block_status()
> to clamp *pnum to the limit where host addresses are no longer
> consecutive, but allowing a NULL file means that *pnum could be
> set to the full length of known-allocated data.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 01/15] block: add default implementations for bdrv_co_get_block_status()

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> From: Manos Pitsidianakis 
> 
> bdrv_co_get_block_status_from_file() and
> bdrv_co_get_block_status_from_backing() set *file to bs->file and
> bs->backing respectively, so that bdrv_co_get_block_status() can recurse
> to them. Future block drivers won't have to duplicate code to implement
> this.
> 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Manos Pitsidianakis 
> Message-Id: <20170629184320.7151-4-el13...@mail.ntua.gr>

Reviewed-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 03/15] block: Add flag to avoid wasted work in bdrv_is_allocated()

[Fam Zheng]

On Mon, 07/03 17:14, Eric Blake wrote:
> @@ -1717,6 +1718,10 @@ int64_t coroutine_fn 
> bdrv_co_get_block_status_from_backing(BlockDriverState *bs,
>   * Drivers not implementing the functionality are assumed to not support
>   * backing files, hence all their sectors are reported as allocated.
>   *
> + * If 'allocation' is true, the caller only cares about allocation
> + * status; this is a hint that a larger 'pnum' result is more
> + * important than including BDRV_BLOCK_OFFSET_VALID in the return.
> + *

This is slightly unintuitive. I would guess "allocation == false" means "I don't
care about the allocation status" but it actually is "I don't care about the
consecutiveness". The "only" semantics is missing in the parameter name.

Maybe rename it to "consecutive" and invert the logic?  I.e. "consecutive ==
true" means BDRV_BLOCK_OFFSET_VALID is wanted.

Sorry for bikeshedding.

Fam

Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Christian Borntraeger]

On 07/04/2017 05:41 AM, QingFeng Hao wrote:
> 
> 
> 在 2017/7/3 18:20, Christian Borntraeger 写道:
>> On 07/03/2017 10:51 AM, QingFeng Hao wrote:
>>> This patch is based on a similar patch from Stefan Hajnoczi -
>>> commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)
>>>
>>> Do not check kvm_eventfds_enabled() when KVM is disabled since it
>>> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
>>> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
>>> qtest or TCG mode.
>>>
>>> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
>>> when KVM is disabled.
>>>
>>> I have tested that virtio-scsi-ccw works under tcg both with and without
>>> iothread.
>>>
>>> This patch fixes qemu-iotests 068, which was accidentally merged early
>>> despite the dependency on ioeventfd.
>>>
>>> Signed-off-by: QingFeng Hao 
>>> ---
>>>   hw/s390x/virtio-ccw.c | 2 +-
>>>   target/s390x/kvm.c| 3 +++
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>>> index 90d37cb9ff..35896eb007 100644
>>> --- a/hw/s390x/virtio-ccw.c
>>> +++ b/hw/s390x/virtio-ccw.c
>>> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice 
>>> *dev, Error **errp)
>>>   sch->cssid, sch->ssid, sch->schid, sch->devno,
>>>   ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>>>
>>> -if (!kvm_eventfds_enabled()) {
>>> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
>>>   dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>>>   }
>>>
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index a3d00196f4..c37f9c3b9e 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier 
>>> *notifier, uint32_t sch,
>>>   .addr = sch,
>>>   .len = 8,
>>>   };
>>> +if (!kvm_enabled()) {
>>> +return 0;
>>> +}
>> thinking more about it. wouldnt it be better to do something like this 
>> instead
>>
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 058ddad..cc47831 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -1240,7 +1240,11 @@ static inline int 
>> s390_assign_subch_ioeventfd(EventNotifier *notifier,
>> uint32_t sch_id, int vq,
>> bool assign)
>>   {
>> -return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
>> +if (kvm_enabled()) {
>> +return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, 
>> assign);
>> +} else {
>> +return 0;
>> +}
>>   }
> Thanks. It makes sense. I'll change it.
>>   static inline void s390_crypto_reset(void)
>>
>>
>> FWIW, it seems that we (s390) do not have a functional equivalent function 
>> as commit
>> 8c56c1a592b5092d91da8d8943c1d6462a6f ("memory: emulate ioeventfd") , so 
>> we will
>> not use the iothreads.
> Ok, but might s390 have skipped the iothread arguments and passed the test, 
> so we could
> still keep this test case?

Yes, lets just fix the test case. We can implement emulated ioeventfds later.