[Qemu-devel] [PATCH] block/iscsi: bump libiscsi requirement to 1.8.0

[Peter Lieven]

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 <address@hidden>
---
 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 <stdio.h>
-#include <iscsi/iscsi.h>
-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 <<EOF
-#include <iscsi/iscsi.h>
-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
-- 
1.7.9.5