| Message ID | 1320030109-20476-1-git-send-email-wuzhy@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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 > >
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
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 >
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);
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(-)