[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 01/10] block/io: add bdrv_co_write_compressed

From: Pavel Butsykin
Subject: Re: [Qemu-devel] [PATCH 01/10] block/io: add bdrv_co_write_compressed
Date: Tue, 17 May 2016 18:01:33 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 16.05.2016 19:52, Eric Blake wrote:
On 05/14/2016 06:45 AM, Denis V. Lunev wrote:
From: Pavel Butsykin <address@hidden>

This patch just adds the interface to the bdrv_co_write_compressed, which
is currently not used but will be useful for safe implementation of the
bdrv_co_write_compressed callback in format drivers.

Signed-off-by: Pavel Butsykin <address@hidden>
Signed-off-by: Denis V. Lunev <address@hidden>
CC: Jeff Cody <address@hidden>
CC: Markus Armbruster <address@hidden>
CC: Eric Blake <address@hidden>
CC: John Snow <address@hidden>
CC: Stefan Hajnoczi <address@hidden>
CC: Kevin Wolf <address@hidden>

+++ b/block/io.c
@@ -1828,8 +1828,8 @@ int bdrv_is_allocated_above(BlockDriverState *top,
      return 0;

-int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
-                          const uint8_t *buf, int nb_sectors)
+int bdrv_co_write_compressed(BlockDriverState *bs, int64_t sector_num,
+                             int nb_sectors, QEMUIOVector *qiov)

As long as we're adding a new public interface, I'd really like us to
make it byte-based.  int64_t sector_num might be better represented as a
byte offset, and int nb_sectors seems redundant with qiov->size.

      BlockDriver *drv = bs->drv;
      int ret;
@@ -1837,7 +1837,7 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t 
      if (!drv) {
          return -ENOMEDIUM;
-    if (!drv->bdrv_write_compressed) {
+    if (!drv->bdrv_co_write_compressed) {
          return -ENOTSUP;
      ret = bdrv_check_request(bs, sector_num, nb_sectors);
@@ -1846,8 +1846,71 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t 

+    assert(qemu_in_coroutine());
+    return drv->bdrv_co_write_compressed(bs, sector_num, nb_sectors, qiov);

Of course, if you make the public interface byte-based, then calling
into the back end will have to scale back to sectors (after first
asserting that we aren't violating the scaling); see how Kevin did it in
commit 166fe9605.

+typedef struct BdrvWriteCompressedCo {
+    BlockDriverState *bs;
+    int64_t sector_num;

Again, I think a byte offset is smarter than a sector number.

Kevin used the byte offset for functions bdrv_driver_pread/_pwrite(It
looks like just an additional interface), which is not the same thing.
Here the bdrv_co/bdrv_write_compressed functions are analogues of the
bdrv_co/bdrv_write functions that still use sectors in the arguments.
So I'm not sure that the interface there needs to be some other.

Kevin, what do you think about this?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]