* [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable @ 2018-01-08 3:09 Fam Zheng 2018-01-08 13:20 ` Peter Lieven 2018-01-09 18:05 ` Paolo Bonzini 0 siblings, 2 replies; 5+ messages in thread From: Fam Zheng @ 2018-01-08 3:09 UTC (permalink / raw) To: qemu-devel; +Cc: Paolo Bonzini, Peter Lieven, qemu-block After the out label there is a check on iTask.task but it is not initialized yet. Fixes: e38bc23454ef763deb4405ebdee6a1081aa00bc8 Signed-off-by: Fam Zheng <famz@redhat.com> --- block/iscsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 5c0a9e55b6..1cb8cc93c5 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -659,8 +659,7 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, int64_t ret; if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { - ret = -EINVAL; - goto out; + return -EINVAL; } /* default to all sectors allocated */ -- 2.14.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable 2018-01-08 3:09 [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable Fam Zheng @ 2018-01-08 13:20 ` Peter Lieven 2018-01-08 15:05 ` Eric Blake 2018-01-09 18:05 ` Paolo Bonzini 1 sibling, 1 reply; 5+ messages in thread From: Peter Lieven @ 2018-01-08 13:20 UTC (permalink / raw) To: Fam Zheng, qemu-devel; +Cc: Paolo Bonzini, qemu-block Am 08.01.2018 um 04:09 schrieb Fam Zheng: > After the out label there is a check on iTask.task but it is not > initialized yet. > > Fixes: e38bc23454ef763deb4405ebdee6a1081aa00bc8 > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/iscsi.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 5c0a9e55b6..1cb8cc93c5 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -659,8 +659,7 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, > int64_t ret; > > if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { > - ret = -EINVAL; > - goto out; > + return -EINVAL; > } > > /* default to all sectors allocated */ If lbpme is 0 we run into the same error. And this is even more likely than an unaligned request. I think the right patch is to move the init of iTask up again where it was: diff --git a/block/iscsi.c b/block/iscsi.c index 5c0a9e5..6a1c537 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -658,6 +658,8 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, uint64_t lba; int64_t ret; + iscsi_co_init_iscsitask(iscsilun, &iTask); + if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { ret = -EINVAL; goto out; @@ -675,7 +677,6 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, lba = sector_qemu2lun(sector_num, iscsilun); - iscsi_co_init_iscsitask(iscsilun, &iTask); qemu_mutex_lock(&iscsilun->mutex); retry: if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun, Peter -- Mit freundlichen Grüßen Peter Lieven ........................................................... KAMP Netzwerkdienste GmbH Vestische Str. 89-91 | 46117 Oberhausen Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40 pl@kamp.de | http://www.kamp.de Geschäftsführer: Heiner Lante | Michael Lante Amtsgericht Duisburg | HRB Nr. 12154 USt-Id-Nr.: DE 120607556 ........................................................... ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable 2018-01-08 13:20 ` Peter Lieven @ 2018-01-08 15:05 ` Eric Blake 0 siblings, 0 replies; 5+ messages in thread From: Eric Blake @ 2018-01-08 15:05 UTC (permalink / raw) To: Peter Lieven, Fam Zheng, qemu-devel; +Cc: Paolo Bonzini, qemu-block [-- Attachment #1: Type: text/plain, Size: 1892 bytes --] On 01/08/2018 07:20 AM, Peter Lieven wrote: > Am 08.01.2018 um 04:09 schrieb Fam Zheng: >> After the out label there is a check on iTask.task but it is not >> initialized yet. >> >> Fixes: e38bc23454ef763deb4405ebdee6a1081aa00bc8 >> Signed-off-by: Fam Zheng <famz@redhat.com> >> --- >> block/iscsi.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> > If lbpme is 0 we run into the same error. And this is even more likely > than an unaligned request. In fact, my byte-based series adds an assertion that unaligned requests aren't possible. > > I think the right patch is to move the init of iTask up again where it was: I had to rebase v7 of my byte-based series on top of the late iTask initialization; moving it back to early initialization makes sense. > > > diff --git a/block/iscsi.c b/block/iscsi.c > index 5c0a9e5..6a1c537 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -658,6 +658,8 @@ static int64_t coroutine_fn > iscsi_co_get_block_status(BlockDriverState *bs, > uint64_t lba; > int64_t ret; > > + iscsi_co_init_iscsitask(iscsilun, &iTask); > + > if (!is_sector_request_lun_aligned(sector_num, nb_sectors, > iscsilun)) { > ret = -EINVAL; > goto out; > @@ -675,7 +677,6 @@ static int64_t coroutine_fn > iscsi_co_get_block_status(BlockDriverState *bs, > > lba = sector_qemu2lun(sector_num, iscsilun); > > - iscsi_co_init_iscsitask(iscsilun, &iTask); > qemu_mutex_lock(&iscsilun->mutex); > retry: > if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun, If you resubmit this as a formal patch, you can add: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable 2018-01-08 3:09 [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable Fam Zheng 2018-01-08 13:20 ` Peter Lieven @ 2018-01-09 18:05 ` Paolo Bonzini 2018-01-09 18:14 ` Eric Blake 1 sibling, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2018-01-09 18:05 UTC (permalink / raw) To: Fam Zheng, qemu-devel; +Cc: Peter Lieven, qemu-block On 08/01/2018 04:09, Fam Zheng wrote: > After the out label there is a check on iTask.task but it is not > initialized yet. > > Fixes: e38bc23454ef763deb4405ebdee6a1081aa00bc8 > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/iscsi.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 5c0a9e55b6..1cb8cc93c5 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -659,8 +659,7 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, > int64_t ret; > > if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { > - ret = -EINVAL; > - goto out; > + return -EINVAL; > } > > /* default to all sectors allocated */ > Queued, thanks. Paolo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable 2018-01-09 18:05 ` Paolo Bonzini @ 2018-01-09 18:14 ` Eric Blake 0 siblings, 0 replies; 5+ messages in thread From: Eric Blake @ 2018-01-09 18:14 UTC (permalink / raw) To: Paolo Bonzini, Fam Zheng, qemu-devel; +Cc: Peter Lieven, qemu-block [-- Attachment #1: Type: text/plain, Size: 1139 bytes --] On 01/09/2018 12:05 PM, Paolo Bonzini wrote: > On 08/01/2018 04:09, Fam Zheng wrote: >> After the out label there is a check on iTask.task but it is not >> initialized yet. >> >> Fixes: e38bc23454ef763deb4405ebdee6a1081aa00bc8 >> Signed-off-by: Fam Zheng <famz@redhat.com> >> --- >> block/iscsi.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/block/iscsi.c b/block/iscsi.c >> index 5c0a9e55b6..1cb8cc93c5 100644 >> --- a/block/iscsi.c >> +++ b/block/iscsi.c >> @@ -659,8 +659,7 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, >> int64_t ret; >> >> if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { >> - ret = -EINVAL; >> - goto out; >> + return -EINVAL; >> } >> >> /* default to all sectors allocated */ >> > > Queued, thanks. I thought we wanted Peter's version, not Fam's. https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg01237.html -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-09 18:14 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-08 3:09 [Qemu-devel] [PATCH] scsi: Don't check uninitialized local variable Fam Zheng 2018-01-08 13:20 ` Peter Lieven 2018-01-08 15:05 ` Eric Blake 2018-01-09 18:05 ` Paolo Bonzini 2018-01-09 18:14 ` Eric Blake
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).