From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57315) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxIMS-0001a5-ED for qemu-devel@nongnu.org; Wed, 18 Jun 2014 12:05:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxIML-0000EV-PB for qemu-devel@nongnu.org; Wed, 18 Jun 2014 12:05:20 -0400 Received: from mail-wi0-x232.google.com ([2a00:1450:400c:c05::232]:48682) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxIML-0000Dz-FB for qemu-devel@nongnu.org; Wed, 18 Jun 2014 12:05:13 -0400 Received: by mail-wi0-f178.google.com with SMTP id n15so1428386wiw.5 for ; Wed, 18 Jun 2014 09:05:12 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <53A1B8B5.40507@redhat.com> Date: Wed, 18 Jun 2014 18:05:09 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1401885206-23144-1-git-send-email-pl@kamp.de> In-Reply-To: <1401885206-23144-1-git-send-email-pl@kamp.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block/iscsi: bump libiscsi requirement to 1.8.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven , qemu-devel@nongnu.org Cc: kwolf@redhat.com, stefanha@redhat.com, ronniesahlberg@gmail.com Il 04/06/2014 14:33, Peter Lieven ha scritto: > This patch lifts the minimum supported libiscsi version from 1.4.0 to > 1.8.0 in preparation to an upcoming patch to dynamically switch between > 10 Byte and 16 Byte CDBs. > > On one this allows us to remove nearly all #ifdefs from the code which > makes the code easier to maintain and read. On the other hand > I would not recommend libiscsi prior to 1.8.0 for production use > because the following important libiscsi fixes for deadlocks and > protocol errors are missing prior to 1.8.0: > > dbe9a1e SOCKET queue cmd PDUs directly in waitpdu queue > 30df192 DATA-OUT set pdu->cmdsn appropriately > 548bd22 ISCSI fix broken send logic in iscsi_scsi_async_command > 14bee10 RECONNECT do not increase CmdSN for immediate PDUs > 1f4a66a PDU queue out PDUs in order of itt. > 562dd46 PDU avoid incrementing itt to 0xffffffff > cd09c0f PDU use serial32 arithmetic for cmdsn, maxcmdsn and expcmdsn. > 89e918e SOCKET validate data_size in in_pdu header > 91267f5 Limit immediate and unsolicited data to FirstBurstLength > > Note that libiscsi 1.8.0 was already released Jan 5th, 2013. > > Signed-off-by: Peter Lieven > --- > block/iscsi.c | 66 --------------------------------------------------------- > configure | 39 +++------------------------------- > 2 files changed, 3 insertions(+), 102 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 1bf9ccd..7394fa6 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -366,17 +366,6 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs, > > lba = sector_qemu2lun(sector_num, iscsilun); > num_sectors = sector_qemu2lun(nb_sectors, iscsilun); > -#if !defined(LIBISCSI_FEATURE_IOVECTOR) > - /* if the iovec only contains one buffer we can pass it directly */ > - if (iov->niov == 1) { > - data = iov->iov[0].iov_base; > - } else { > - size_t size = MIN(nb_sectors * BDRV_SECTOR_SIZE, iov->size); > - buf = g_malloc(size); > - qemu_iovec_to_buf(iov, 0, buf, size); > - data = buf; > - } > -#endif > iscsi_co_init_iscsitask(iscsilun, &iTask); > retry: > iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba, > @@ -387,10 +376,8 @@ retry: > g_free(buf); > return -ENOMEM; > } > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > scsi_task_set_iov_out(iTask.task, (struct scsi_iovec *) iov->iov, > iov->niov); > -#endif > while (!iTask.complete) { > iscsi_set_events(iscsilun); > qemu_coroutine_yield(); > @@ -417,7 +404,6 @@ retry: > return 0; > } > > - > static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, > int64_t sector_num, int nb_sectors) > { > @@ -430,9 +416,6 @@ static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, > sector_num / iscsilun->cluster_sectors) == size); > } > > - > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > - > static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, > int64_t sector_num, > int nb_sectors, int *pnum) > @@ -530,9 +513,6 @@ out: > return ret; > } > > -#endif /* LIBISCSI_FEATURE_IOVECTOR */ > - > - > static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, > int64_t sector_num, int nb_sectors, > QEMUIOVector *iov) > @@ -541,15 +521,11 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, > struct IscsiTask iTask; > uint64_t lba; > uint32_t num_sectors; > -#if !defined(LIBISCSI_FEATURE_IOVECTOR) > - int i; > -#endif > > if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { > return -EINVAL; > } > > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > if (iscsilun->lbprz && nb_sectors >= ISCSI_CHECKALLOC_THRES && > !iscsi_allocationmap_is_allocated(iscsilun, sector_num, nb_sectors)) { > int64_t ret; > @@ -563,7 +539,6 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, > return 0; > } > } > -#endif > > lba = sector_qemu2lun(sector_num, iscsilun); > num_sectors = sector_qemu2lun(nb_sectors, iscsilun); > @@ -581,24 +556,14 @@ retry: > iTask.task = iscsi_read10_task(iscsilun->iscsi, iscsilun->lun, lba, > num_sectors * iscsilun->block_size, > iscsilun->block_size, > -#if !defined(CONFIG_LIBISCSI_1_4) /* API change from 1.4.0 to 1.5.0 */ > 0, 0, 0, 0, 0, > -#endif > iscsi_co_generic_cb, &iTask); > break; > } > if (iTask.task == NULL) { > return -ENOMEM; > } > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > scsi_task_set_iov_in(iTask.task, (struct scsi_iovec *) iov->iov, iov->niov); > -#else > - for (i = 0; i < iov->niov; i++) { > - scsi_task_add_data_in_buffer(iTask.task, > - iov->iov[i].iov_len, > - iov->iov[i].iov_base); > - } > -#endif > > while (!iTask.complete) { > iscsi_set_events(iscsilun); > @@ -753,18 +718,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, > data.data = acb->ioh->dxferp; > data.size = acb->ioh->dxfer_len; > } else { > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > scsi_task_set_iov_out(acb->task, > (struct scsi_iovec *) acb->ioh->dxferp, > acb->ioh->iovec_count); > -#else > - struct iovec *iov = (struct iovec *)acb->ioh->dxferp; > - > - acb->buf = g_malloc(acb->ioh->dxfer_len); > - data.data = acb->buf; > - data.size = iov_to_buf(iov, acb->ioh->iovec_count, 0, > - acb->buf, acb->ioh->dxfer_len); > -#endif > } > } > > @@ -784,20 +740,9 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, > acb->ioh->dxfer_len, > acb->ioh->dxferp); > } else { > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > scsi_task_set_iov_in(acb->task, > (struct scsi_iovec *) acb->ioh->dxferp, > acb->ioh->iovec_count); > -#else > - int i; > - for (i = 0; i < acb->ioh->iovec_count; i++) { > - struct iovec *iov = (struct iovec *)acb->ioh->dxferp; > - > - scsi_task_add_data_in_buffer(acb->task, > - iov[i].iov_len, > - iov[i].iov_base); > - } > -#endif > } > } > > @@ -806,7 +751,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, > return &acb->common; > } > > - > static void ioctl_cb(void *opaque, int status) > { > int *p_status = opaque; > @@ -912,7 +856,6 @@ retry: > } > > #if defined(SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED) > - > static int > coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, > int nb_sectors, BdrvRequestFlags flags) > @@ -989,7 +932,6 @@ retry: > > return 0; > } > - > #endif /* SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED */ > > static void parse_chap(struct iscsi_context *iscsi, const char *target, > @@ -1101,7 +1043,6 @@ static char *parse_initiator_name(const char *target) > return iscsi_name; > } > > -#if defined(LIBISCSI_FEATURE_NOP_COUNTER) > static void iscsi_nop_timed_event(void *opaque) > { > IscsiLun *iscsilun = opaque; > @@ -1119,7 +1060,6 @@ static void iscsi_nop_timed_event(void *opaque) > timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL); > iscsi_set_events(iscsilun); > } > -#endif > > static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp) > { > @@ -1258,14 +1198,12 @@ static void iscsi_attach_aio_context(BlockDriverState *bs, > iscsilun->aio_context = new_context; > iscsi_set_events(iscsilun); > > -#if defined(LIBISCSI_FEATURE_NOP_COUNTER) > /* Set up a timer for sending out iSCSI NOPs */ > iscsilun->nop_timer = aio_timer_new(iscsilun->aio_context, > QEMU_CLOCK_REALTIME, SCALE_MS, > iscsi_nop_timed_event, iscsilun); > timer_mod(iscsilun->nop_timer, > qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL); > -#endif > } > > /* > @@ -1457,13 +1395,11 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, > iscsilun->bl.opt_unmap_gran * iscsilun->block_size <= 16 * 1024 * 1024) { > iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran * > iscsilun->block_size) >> BDRV_SECTOR_BITS; > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > if (iscsilun->lbprz && !(bs->open_flags & BDRV_O_NOCACHE)) { > iscsilun->allocationmap = > bitmap_new(DIV_ROUND_UP(bs->total_sectors, > iscsilun->cluster_sectors)); > } > -#endif > } > > out: > @@ -1652,9 +1588,7 @@ static BlockDriver bdrv_iscsi = { > .bdrv_truncate = iscsi_truncate, > .bdrv_refresh_limits = iscsi_refresh_limits, > > -#if defined(LIBISCSI_FEATURE_IOVECTOR) > .bdrv_co_get_block_status = iscsi_co_get_block_status, > -#endif > .bdrv_co_discard = iscsi_co_discard, > #if defined(SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED) > .bdrv_co_write_zeroes = iscsi_co_write_zeroes, > diff --git a/configure b/configure > index 69b9f56..1ef998e 100755 > --- a/configure > +++ b/configure > @@ -3346,46 +3346,20 @@ if compile_prog "" "" ; then > fi > > ########################################## > -# Do we have libiscsi > -# We check for iscsi_write16_sync() to make sure we have a > -# at least version 1.4.0 of libiscsi. > +# Do we have libiscsi >= 1.8.0 > if test "$libiscsi" != "no" ; then > - cat > $TMPC << EOF > -#include > -#include > -int main(void) { iscsi_write16_sync(NULL,0,0,NULL,0,0,0,0,0,0,0); return 0; } > -EOF > - if $pkg_config --atleast-version=1.7.0 libiscsi; then > + if $pkg_config --atleast-version=1.8.0 libiscsi; then > libiscsi="yes" > libiscsi_cflags=$($pkg_config --cflags libiscsi) > libiscsi_libs=$($pkg_config --libs libiscsi) > - elif compile_prog "" "-liscsi" ; then > - libiscsi="yes" > - libiscsi_libs="-liscsi" > else > if test "$libiscsi" = "yes" ; then > - feature_not_found "libiscsi" "Install libiscsi devel" > + feature_not_found "libiscsi" "Install libiscsi >= 1.8.0" > fi > libiscsi="no" > fi > fi > > -# We also need to know the API version because there was an > -# API change from 1.4.0 to 1.5.0. > -if test "$libiscsi" = "yes"; then > - cat >$TMPC < -#include > -int main(void) > -{ > - iscsi_read10_task(0, 0, 0, 0, 0, 0, 0); > - return 0; > -} > -EOF > - if compile_prog "" "-liscsi"; then > - libiscsi_version="1.4.0" > - fi > -fi > - > ########################################## > # Do we need libm > cat > $TMPC << EOF > @@ -4160,11 +4134,7 @@ echo "nss used $smartcard_nss" > echo "libusb $libusb" > echo "usb net redir $usb_redir" > echo "GLX support $glx" > -if test "$libiscsi_version" = "1.4.0"; then > -echo "libiscsi support $libiscsi (1.4.0)" > -else > echo "libiscsi support $libiscsi" > -fi > echo "libnfs support $libnfs" > echo "build guest agent $guest_agent" > echo "QGA VSS support $guest_agent_with_vss" > @@ -4519,9 +4489,6 @@ fi > > if test "$libiscsi" = "yes" ; then > echo "CONFIG_LIBISCSI=m" >> $config_host_mak > - if test "$libiscsi_version" = "1.4.0"; then > - echo "CONFIG_LIBISCSI_1_4=y" >> $config_host_mak > - fi > echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak > echo "LIBISCSI_LIBS=$libiscsi_libs" >> $config_host_mak > fi > Applied now, with further simplifications to bump to 1.9.0. Paolo