[Qemu-devel] [PATCH] qed: adjust the way to get nb_sectors

[Qemu-devel] [PATCH] qed: adjust the way to get nb_sectors

[Zhi Yong Wu]

It is better to use qiov.size in qed-table.c to get nb_sectors than iov.iov_len.

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 block/qed-table.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/qed-table.c b/block/qed-table.c
index f31f9ff..8ee8443 100644
--- a/block/qed-table.c
+++ b/block/qed-table.c
@@ -29,7 +29,7 @@ static void qed_read_table_cb(void *opaque, int ret)
 {
     QEDReadTableCB *read_table_cb = opaque;
     QEDTable *table = read_table_cb->table;
-    int noffsets = read_table_cb->iov.iov_len / sizeof(uint64_t);
+    int noffsets = read_table_cb->qiov.size / sizeof(uint64_t);
     int i;
 
     /* Handle I/O error */
@@ -65,7 +65,7 @@ static void qed_read_table(BDRVQEDState *s, uint64_t offset, QEDTable *table,
 
     qemu_iovec_init_external(qiov, &read_table_cb->iov, 1);
     aiocb = bdrv_aio_readv(s->bs->file, offset / BDRV_SECTOR_SIZE, qiov,
-                           read_table_cb->iov.iov_len / BDRV_SECTOR_SIZE,
+                           qiov->size / BDRV_SECTOR_SIZE,
                            qed_read_table_cb, read_table_cb);
     if (!aiocb) {
         qed_read_table_cb(read_table_cb, -EIO);
@@ -160,7 +160,7 @@ static void qed_write_table(BDRVQEDState *s, uint64_t offset, QEDTable *table,
 
     aiocb = bdrv_aio_writev(s->bs->file, offset / BDRV_SECTOR_SIZE,
                             &write_table_cb->qiov,
-                            write_table_cb->iov.iov_len / BDRV_SECTOR_SIZE,
+                            write_table_cb->qiov.size / BDRV_SECTOR_SIZE,
                             qed_write_table_cb, write_table_cb);
     if (!aiocb) {
         qed_write_table_cb(write_table_cb, -EIO);
-- 
1.7.6

Re: [Qemu-devel] [PATCH] qed: adjust the way to get nb_sectors

[Kevin Wolf]

Am 31.10.2011 04:01, schrieb Zhi Yong Wu:
> It is better to use qiov.size in qed-table.c to get nb_sectors than iov.iov_len.
> 
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

The commit message should probably say why it's better. Not saying
otherwise, but I can't see the different at the first sight.

Kevin

Re: [Qemu-devel] [PATCH] qed: adjust the way to get nb_sectors

[Zhi Yong Wu]

On Mon, Oct 31, 2011 at 4:10 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 31.10.2011 04:01, schrieb Zhi Yong Wu:
>> It is better to use qiov.size in qed-table.c to get nb_sectors than iov.iov_len.
>>
>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> The commit message should probably say why it's better. Not saying
> otherwise, but I can't see the different at the first sight.
They are equal, but if the number of iov isn't ONE, they will be not
equal. qiov.size contains the sum of all iov's size while iov.iov_len
is only the size of one iov. Although in current scenario, they are
equal, but i think that it is better if qiov.size is used.

's >
> Kevin
>
>



-- 
Regards,

Zhi Yong Wu

Re: [Qemu-devel] [PATCH] qed: adjust the way to get nb_sectors

[Stefan Hajnoczi]

On Mon, Oct 31, 2011 at 8:25 AM, Zhi Yong Wu <zwu.kernel@gmail.com> wrote:
> On Mon, Oct 31, 2011 at 4:10 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 31.10.2011 04:01, schrieb Zhi Yong Wu:
>>> It is better to use qiov.size in qed-table.c to get nb_sectors than iov.iov_len.
>>>
>>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>
>> The commit message should probably say why it's better. Not saying
>> otherwise, but I can't see the different at the first sight.
> They are equal, but if the number of iov isn't ONE, they will be not
> equal. qiov.size contains the sum of all iov's size while iov.iov_len
> is only the size of one iov. Although in current scenario, they are
> equal, but i think that it is better if qiov.size is used.

I see your reasoning.  Especially in qed_read_table_cb() it's nice to
use qiov->size because that function doesn't obviously use a single
struct iovec.

If you want to change it I agree but please send a patch with a proper
explanation that mentions that this is purely a refactoring (does not
change behavior) and why.

Stefan

Re: [Qemu-devel] [PATCH] qed: adjust the way to get nb_sectors

[Zhi Yong Wu]

On Mon, Oct 31, 2011 at 02:11:00PM +0000, Stefan Hajnoczi wrote:
>From: Stefan Hajnoczi <stefanha@gmail.com>
>To: Zhi Yong Wu <zwu.kernel@gmail.com>
>Cc: Kevin Wolf <kwolf@redhat.com>, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>,
> qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com
>Content-Type: text/plain; charset=ISO-8859-1
>x-cbid: 11103114-6148-0000-0000-000000CC1760
>X-IBM-ISS-SpamDetectors: Score=0; BY=0; FL=0; FP=0; FZ=0; HX=0; KW=0; PH=0;
> SC=0; ST=0; TS=0; UL=0; ISC=
>X-IBM-ISS-DetailInfo: BY=3.00000227; HX=3.00000175; KW=3.00000007;
> PH=3.00000001; SC=3.00000001; SDB=6.00083535; UDB=6.00022970;
> UTC=2011-10-31 14:11:03
>X-Xagent-From: stefanha@gmail.com
>X-Xagent-To: wuzhy@linux.vnet.ibm.com
>X-Xagent-Gateway: vmsdvma.vnet.ibm.com (XAGENTU6 at VMSDVMA)
>
>On Mon, Oct 31, 2011 at 8:25 AM, Zhi Yong Wu <zwu.kernel@gmail.com> wrote:
>> On Mon, Oct 31, 2011 at 4:10 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> Am 31.10.2011 04:01, schrieb Zhi Yong Wu:
>>>> It is better to use qiov.size in qed-table.c to get nb_sectors than iov.iov_len.
>>>>
>>>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>>
>>> The commit message should probably say why it's better. Not saying
>>> otherwise, but I can't see the different at the first sight.
>> They are equal, but if the number of iov isn't ONE, they will be not
>> equal. qiov.size contains the sum of all iov's size while iov.iov_len
>> is only the size of one iov. Although in current scenario, they are
>> equal, but i think that it is better if qiov.size is used.
>
>I see your reasoning.  Especially in qed_read_table_cb() it's nice to
>use qiov->size because that function doesn't obviously use a single
>struct iovec.
>
>If you want to change it I agree but please send a patch with a proper
>explanation that mentions that this is purely a refactoring (does not
>change behavior) and why.
OK.
>
>Stefan
>