[Qemu-devel] [PULL 0/6] Misc patches for 2016-09-22

[Qemu-devel] [PULL 0/6] Misc patches for 2016-09-22

[Paolo Bonzini]

The following changes since commit a008535b9fa396226ff9cf78b8ac5f3584bda58e:

  build-sys: fix make install regression (2016-09-20 11:32:43 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to a84d5bb03ba81b7eccf49e4b7dd7a0ecba47efdd:

  kvm: fix events.flags (KVM_VCPUEVENT_VALID_SMM) overwritten by 0 (2016-09-22 18:44:06 +0200)

----------------------------------------------------------------
* More KVM LAPIC fixes
* fix divide-by-zero regression on libiscsi SG devices
* fix qemu-char segfault
* add scripts/show-fixed-bugs.sh

----------------------------------------------------------------
Dr. David Alan Gilbert (1):
      kvm: apic: set APIC base as part of kvm_apic_put

Eric Blake (1):
      iscsi: Fix divide-by-zero regression on raw SG devices

Herongguang (Stephen) (1):
      kvm: fix events.flags (KVM_VCPUEVENT_VALID_SMM) overwritten by 0

Lin Ma (1):
      msmouse: Fix segfault caused by free the chr before chardev cleanup.

Paolo Bonzini (1):
      target-i386: introduce kvm_put_one_msr

Thomas Huth (1):
      scripts: Add a script to check for bug URLs in the git log

 backends/msmouse.c         |  1 -
 block/iscsi.c              | 14 ++++---
 hw/i386/kvm/apic.c         |  2 +
 scripts/show-fixed-bugs.sh | 91 ++++++++++++++++++++++++++++++++++++++++++++++
 target-i386/kvm.c          | 30 ++++++++++-----
 target-i386/kvm_i386.h     |  2 +
 6 files changed, 124 insertions(+), 16 deletions(-)
 create mode 100755 scripts/show-fixed-bugs.sh
-- 
2.7.4

[Qemu-devel] [PULL 1/6] target-i386: introduce kvm_put_one_msr

[Paolo Bonzini]

Avoid further code duplication in the next patch.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/kvm.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f1ad805..c57b01b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1532,6 +1532,14 @@ static void kvm_msr_entry_add(X86CPU *cpu, uint32_t index, uint64_t value)
     msrs->nmsrs++;
 }
 
+static int kvm_put_one_msr(X86CPU *cpu, int index, uint64_t value)
+{
+    kvm_msr_buf_reset(cpu);
+    kvm_msr_entry_add(cpu, index, value);
+
+    return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
+}
+
 static int kvm_put_tscdeadline_msr(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
@@ -1541,10 +1549,7 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu)
         return 0;
     }
 
-    kvm_msr_buf_reset(cpu);
-    kvm_msr_entry_add(cpu, MSR_IA32_TSCDEADLINE, env->tsc_deadline);
-
-    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
+    ret = kvm_put_one_msr(cpu, MSR_IA32_TSCDEADLINE, env->tsc_deadline);
     if (ret < 0) {
         return ret;
     }
@@ -1567,11 +1572,8 @@ static int kvm_put_msr_feature_control(X86CPU *cpu)
         return 0;
     }
 
-    kvm_msr_buf_reset(cpu);
-    kvm_msr_entry_add(cpu, MSR_IA32_FEATURE_CONTROL,
-                      cpu->env.msr_ia32_feature_control);
-
-    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
+    ret = kvm_put_one_msr(cpu, MSR_IA32_FEATURE_CONTROL,
+                          cpu->env.msr_ia32_feature_control);
     if (ret < 0) {
         return ret;
     }
-- 
2.7.4

[Qemu-devel] [PULL 2/6] kvm: apic: set APIC base as part of kvm_apic_put

[Paolo Bonzini]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The parsing of KVM_SET_LAPIC's input depends on the current value of the
APIC base MSR---which indeed is stored in APICCommonState---but for historical
reasons APIC base is set through KVM_SET_SREGS together with cr8 (which is
really just the APIC TPR) and the actual "special CPU registers".

APIC base must now be set before the actual LAPIC registers, so do that
in kvm_apic_put.  It will be set again to the same value with KVM_SET_SREGS,
but that's not a big issue.

This only happens since Linux 4.8, which checks for x2apic mode in
KVM_SET_LAPIC.  However it's really a QEMU bug; until the recent
commit 78d6a05 ("x86/lapic: Load LAPIC state at post_load", 2016-09-13)
QEMU was indeed setting APIC base (via KVM_SET_SREGS) before the other
LAPIC registers.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/kvm/apic.c     | 2 ++
 target-i386/kvm.c      | 8 ++++++++
 target-i386/kvm_i386.h | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index feb0002..f57fed1 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -15,6 +15,7 @@
 #include "hw/i386/apic_internal.h"
 #include "hw/pci/msi.h"
 #include "sysemu/kvm.h"
+#include "target-i386/kvm_i386.h"
 
 static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
                                     int reg_id, uint32_t val)
@@ -130,6 +131,7 @@ static void kvm_apic_put(void *data)
     struct kvm_lapic_state kapic;
     int ret;
 
+    kvm_put_apicbase(s->cpu, s->apicbase);
     kvm_put_apic_state(s, &kapic);
 
     ret = kvm_vcpu_ioctl(CPU(s->cpu), KVM_SET_LAPIC, &kapic);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index c57b01b..f236daf 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1540,6 +1540,14 @@ static int kvm_put_one_msr(X86CPU *cpu, int index, uint64_t value)
     return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
 }
 
+void kvm_put_apicbase(X86CPU *cpu, uint64_t value)
+{
+    int ret;
+
+    ret = kvm_put_one_msr(cpu, MSR_IA32_APICBASE, value);
+    assert(ret == 1);
+}
+
 static int kvm_put_tscdeadline_msr(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 42b00af..36407e0 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -41,4 +41,6 @@ int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
 int kvm_device_msix_assign(KVMState *s, uint32_t dev_id);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
+void kvm_put_apicbase(X86CPU *cpu, uint64_t value);
+
 #endif
-- 
2.7.4

[Qemu-devel] [PULL 3/6] iscsi: Fix divide-by-zero regression on raw SG devices

[Paolo Bonzini]

From: Eric Blake <eblake@redhat.com>

When qemu uses iscsi devices in sg mode, iscsilun->block_size
is left at 0.  Prior to commits cf081fca and similar, when
block limits were tracked in sectors, this did not matter:
various block limits were just left at 0.  But when we started
scaling by block size, this caused SIGFPE.

Then, in a later patch, commit a5b8dd2c added an assertion to
bdrv_open_common() that request_alignment is always non-zero;
which was not true for SG mode.  Rather than relax that assertion,
we can just provide a sane value (we don't know of any SG device
with a block size smaller than qemu's default sizing of 512 bytes).

One possible solution for SG mode is to just blindly skip ALL
of iscsi_refresh_limits(), since we already short circuit so
many other things in sg mode.  But this patch takes a slightly
more conservative approach, and merely guarantees that scaling
will succeed, while still using multiples of the original size
where possible.  Resulting limits may still be zero in SG mode
(that is, we mostly only fix block_size used as a denominator
or which affect assertions, not all uses).

Reported-by: Holger Schranz <holger@fam-schranz.de>
Signed-off-by: Eric Blake <eblake@redhat.com>
CC: qemu-stable@nongnu.org

Message-Id: <1473283640-15756-1-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 95ce9e1..c01e955 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1813,19 +1813,23 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
 
     IscsiLun *iscsilun = bs->opaque;
     uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;
+    unsigned int block_size = MIN_NON_ZERO(BDRV_SECTOR_SIZE,
+                                           iscsilun->block_size);
 
-    bs->bl.request_alignment = iscsilun->block_size;
+    assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bs->sg);
+
+    bs->bl.request_alignment = block_size;
 
     if (iscsilun->bl.max_xfer_len) {
         max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len);
     }
 
-    if (max_xfer_len * iscsilun->block_size < INT_MAX) {
+    if (max_xfer_len * block_size < INT_MAX) {
         bs->bl.max_transfer = max_xfer_len * iscsilun->block_size;
     }
 
     if (iscsilun->lbp.lbpu) {
-        if (iscsilun->bl.max_unmap < 0xffffffff / iscsilun->block_size) {
+        if (iscsilun->bl.max_unmap < 0xffffffff / block_size) {
             bs->bl.max_pdiscard =
                 iscsilun->bl.max_unmap * iscsilun->block_size;
         }
@@ -1835,7 +1839,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
         bs->bl.pdiscard_alignment = iscsilun->block_size;
     }
 
-    if (iscsilun->bl.max_ws_len < 0xffffffff / iscsilun->block_size) {
+    if (iscsilun->bl.max_ws_len < 0xffffffff / block_size) {
         bs->bl.max_pwrite_zeroes =
             iscsilun->bl.max_ws_len * iscsilun->block_size;
     }
@@ -1846,7 +1850,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
         bs->bl.pwrite_zeroes_alignment = iscsilun->block_size;
     }
     if (iscsilun->bl.opt_xfer_len &&
-        iscsilun->bl.opt_xfer_len < INT_MAX / iscsilun->block_size) {
+        iscsilun->bl.opt_xfer_len < INT_MAX / block_size) {
         bs->bl.opt_transfer = pow2floor(iscsilun->bl.opt_xfer_len *
                                         iscsilun->block_size);
     }
-- 
2.7.4

[Qemu-devel] [PULL 4/6] msmouse: Fix segfault caused by free the chr before chardev cleanup.

[Paolo Bonzini]

From: Lin Ma <lma@suse.com>

Segfault happens when leaving qemu with msmouse backend:

 #0  0x00007fa8526ac975 in raise () at /lib64/libc.so.6
 #1  0x00007fa8526add8a in abort () at /lib64/libc.so.6
 #2  0x0000558be78846ab in error_exit (err=16, msg=0x558be799da10 ...
 #3  0x0000558be7884717 in qemu_mutex_destroy (mutex=0x558be93be750) at ...
 #4  0x0000558be7549951 in qemu_chr_free_common (chr=0x558be93be750) at ...
 #5  0x0000558be754999c in qemu_chr_free (chr=0x558be93be750) at ...
 #6  0x0000558be7549a20 in qemu_chr_delete (chr=0x558be93be750) at ...
 #7  0x0000558be754a8ef in qemu_chr_cleanup () at qemu-char.c:4643
 #8  0x0000558be755843e in main (argc=5, argv=0x7ffe925d7118, ...

The chr was freed by msmouse close callback before chardev cleanup,
Then qemu_mutex_destroy triggered raise().

Because freeing chr is handled by qemu_chr_free_common, Remove the free from
msmouse_chr_close to avoid double free.

Fixes: c1111a24a3358ecd2f17be7c8b117cfe8bc5e5f8
Cc: qemu-stable@nongnu.org
Signed-off-by: Lin Ma <lma@suse.com>
Message-Id: <20160915143158.4796-1-lma@suse.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/msmouse.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/backends/msmouse.c b/backends/msmouse.c
index aceb6dc..85d08f7 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -139,7 +139,6 @@ static void msmouse_chr_close (struct CharDriverState *chr)
 
     qemu_input_handler_unregister(mouse->hs);
     g_free(mouse);
-    g_free(chr);
 }
 
 static QemuInputHandler msmouse_handler = {
-- 
2.7.4

[Qemu-devel] [PULL 5/6] scripts: Add a script to check for bug URLs in the git log

[Paolo Bonzini]

From: Thomas Huth <thuth@redhat.com>

Basic idea of this script is to check the git log for URLs
to the QEMU bugtracker at launchpad.net and to figure out
whether the related bug has been marked there as "Fix released"
(i.e. closed) already. So this script can e.g. be used after
each public release of QEMU to check whether there are any
bug tickets that could be moved from "Fix committed" (or another
state if the author of the patch forgot to update the bug ticket)
to "Fix released".

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1474486942-18754-1-git-send-email-thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/show-fixed-bugs.sh | 91 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100755 scripts/show-fixed-bugs.sh

diff --git a/scripts/show-fixed-bugs.sh b/scripts/show-fixed-bugs.sh
new file mode 100755
index 0000000..36f3068
--- /dev/null
+++ b/scripts/show-fixed-bugs.sh
@@ -0,0 +1,91 @@
+#!/bin/sh
+
+# This script checks the git log for URLs to the QEMU launchpad bugtracker
+# and optionally checks whether the corresponding bugs are not closed yet.
+
+show_help () {
+    echo "Usage:"
+    echo "  -s <commit>  : Start searching at this commit"
+    echo "  -e <commit>  : End searching at this commit"
+    echo "  -c           : Check if bugs are still open"
+    echo "  -b           : Open bugs in browser"
+}
+
+while getopts "s:e:cbh" opt; do
+   case "$opt" in
+    s)  start="$OPTARG" ;;
+    e)  end="$OPTARG" ;;
+    c)  check_if_open=1 ;;
+    b)  show_in_browser=1 ;;
+    h)  show_help ; exit 0 ;;
+    *)   echo "Use -h for help." ; exit 1 ;;
+   esac
+done
+
+if [ "x$start" = "x" ]; then
+    start=`git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1`
+fi
+if [ "x$end" = "x" ]; then
+    end=`git tag -l  'v[0-9]*\.[0-9]*\.0' | tail -n 1`
+fi
+
+if [ "x$start" = "x" ] || [ "x$end" = "x" ]; then
+    echo "Could not determine start or end revision ... Please note that this"
+    echo "script must be run from a checked out git repository of QEMU."
+    exit 1
+fi
+
+echo "Searching git log for bugs in the range $start..$end"
+
+urlstr='https://bugs.launchpad.net/\(bugs\|qemu/+bug\)/'
+bug_urls=`git log $start..$end \
+  | sed -n '\,'"$urlstr"', s,\(.*\)\('"$urlstr"'\)\([0-9]*\).*,\2\4,p' \
+  | sort -u`
+
+echo Found bug URLs:
+for i in $bug_urls ; do echo " $i" ; done
+
+if [ "x$check_if_open" = "x1" ]; then
+    echo
+    echo "Checking which ones are still open..."
+    for i in $bug_urls ; do
+        if ! curl -s -L "$i" | grep "value status" | grep -q "Fix Released" ; then
+            echo " $i"
+            final_bug_urls="$final_bug_urls $i"
+        fi
+    done
+else
+    final_bug_urls=$bug_urls
+fi
+
+if [ "x$final_bug_urls" = "x" ]; then
+    echo "No open bugs found."
+elif [ "x$show_in_browser" = "x1" ]; then
+    # Try to determine which browser we should use
+    if [ "x$BROWSER" != "x" ]; then
+        bugbrowser="$BROWSER"
+    elif command -v xdg-open >/dev/null 2>&1; then
+        bugbrowser=xdg-open
+    elif command -v gnome-open >/dev/null 2>&1; then
+        bugbrowser=gnome-open
+    elif [ "`uname`" = "Darwin" ]; then
+        bugbrowser=open
+    elif command -v sensible-browser >/dev/null 2>&1; then
+        bugbrowser=sensible-browser
+    else
+        echo "Please set the BROWSER variable to the browser of your choice."
+        exit 1
+    fi
+    # Now show the bugs in the browser
+    first=1
+    for i in $final_bug_urls; do
+        "$bugbrowser" "$i"
+        if [ $first = 1 ]; then
+            # if it is the first entry, give the browser some time to start
+            # (to avoid messages like "Firefox is already running, but is
+            # not responding...")
+            sleep 4
+            first=0
+        fi
+    done
+fi
-- 
2.7.4

[Qemu-devel] [PULL 6/6] kvm: fix events.flags (KVM_VCPUEVENT_VALID_SMM) overwritten by 0

[Paolo Bonzini]

From: "Herongguang (Stephen)" <herongguang.he@huawei.com>

Fix events.flags (KVM_VCPUEVENT_VALID_SMM) overwritten by 0.

Signed-off-by: He Rongguang <herongguang.he@huawei.com>
Message-Id: <57E38EAC.3020108@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f236daf..a0e42b2 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2452,6 +2452,7 @@ static int kvm_put_vcpu_events(X86CPU *cpu, int level)
     events.nmi.pad = 0;
 
     events.sipi_vector = env->sipi_vector;
+    events.flags = 0;
 
     if (has_msr_smbase) {
         events.smi.smm = !!(env->hflags & HF_SMM_MASK);
@@ -2471,7 +2472,6 @@ static int kvm_put_vcpu_events(X86CPU *cpu, int level)
         events.flags |= KVM_VCPUEVENT_VALID_SMM;
     }
 
-    events.flags = 0;
     if (level >= KVM_PUT_RESET_STATE) {
         events.flags |=
             KVM_VCPUEVENT_VALID_NMI_PENDING | KVM_VCPUEVENT_VALID_SIPI_VECTOR;
-- 
2.7.4

Re: [Qemu-devel] [PULL 3/6] iscsi: Fix divide-by-zero regression on raw SG devices

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

On 09/22/2016 11:52 AM, Paolo Bonzini wrote:
> From: Eric Blake <eblake@redhat.com>
> 
> When qemu uses iscsi devices in sg mode, iscsilun->block_size
> is left at 0.  Prior to commits cf081fca and similar, when
> block limits were tracked in sectors, this did not matter:
> various block limits were just left at 0.  But when we started
> scaling by block size, this caused SIGFPE.
> 

> +++ b/block/iscsi.c
> @@ -1813,19 +1813,23 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
>  
>      IscsiLun *iscsilun = bs->opaque;
>      uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;
> +    unsigned int block_size = MIN_NON_ZERO(BDRV_SECTOR_SIZE,
> +                                           iscsilun->block_size);
>  
> -    bs->bl.request_alignment = iscsilun->block_size;
> +    assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bs->sg);
> +
> +    bs->bl.request_alignment = block_size;

Ouch. If iscsilun->block_size is 4k, this ends up changing
bs->bl.request_alignment to 512, which is wrong. :(

If I'm in time, let's drop this from the pull request, and I'll send a
followup patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PULL 0/6] Misc patches for 2016-09-22

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

On 09/22/2016 11:52 AM, Paolo Bonzini wrote:
> The following changes since commit a008535b9fa396226ff9cf78b8ac5f3584bda58e:
> 
>   build-sys: fix make install regression (2016-09-20 11:32:43 +0100)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git tags/for-upstream
> 
> for you to fetch changes up to a84d5bb03ba81b7eccf49e4b7dd7a0ecba47efdd:
> 
>   kvm: fix events.flags (KVM_VCPUEVENT_VALID_SMM) overwritten by 0 (2016-09-22 18:44:06 +0200)
> 
> ----------------------------------------------------------------
> * More KVM LAPIC fixes
> * fix divide-by-zero regression on libiscsi SG devices
> * fix qemu-char segfault
> * add scripts/show-fixed-bugs.sh
> 
> ----------------------------------------------------------------
> Dr. David Alan Gilbert (1):
>       kvm: apic: set APIC base as part of kvm_apic_put
> 
> Eric Blake (1):
>       iscsi: Fix divide-by-zero regression on raw SG devices

Heads up that this patch is faulty, if you want to hold off for a v2
pull request.

> 
> Herongguang (Stephen) (1):
>       kvm: fix events.flags (KVM_VCPUEVENT_VALID_SMM) overwritten by 0
> 
> Lin Ma (1):
>       msmouse: Fix segfault caused by free the chr before chardev cleanup.
> 
> Paolo Bonzini (1):
>       target-i386: introduce kvm_put_one_msr
> 
> Thomas Huth (1):
>       scripts: Add a script to check for bug URLs in the git log
> 
>  backends/msmouse.c         |  1 -
>  block/iscsi.c              | 14 ++++---
>  hw/i386/kvm/apic.c         |  2 +
>  scripts/show-fixed-bugs.sh | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>  target-i386/kvm.c          | 30 ++++++++++-----
>  target-i386/kvm_i386.h     |  2 +
>  6 files changed, 124 insertions(+), 16 deletions(-)
>  create mode 100755 scripts/show-fixed-bugs.sh
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PULL 3/6] iscsi: Fix divide-by-zero regression on raw SG devices

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]



On 22/09/2016 19:34, Eric Blake wrote:
> On 09/22/2016 11:52 AM, Paolo Bonzini wrote:
>> From: Eric Blake <eblake@redhat.com>
>>
>> When qemu uses iscsi devices in sg mode, iscsilun->block_size
>> is left at 0.  Prior to commits cf081fca and similar, when
>> block limits were tracked in sectors, this did not matter:
>> various block limits were just left at 0.  But when we started
>> scaling by block size, this caused SIGFPE.
>>
> 
>> +++ b/block/iscsi.c
>> @@ -1813,19 +1813,23 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
>>  
>>      IscsiLun *iscsilun = bs->opaque;
>>      uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;
>> +    unsigned int block_size = MIN_NON_ZERO(BDRV_SECTOR_SIZE,
>> +                                           iscsilun->block_size);
>>  
>> -    bs->bl.request_alignment = iscsilun->block_size;
>> +    assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bs->sg);
>> +
>> +    bs->bl.request_alignment = block_size;
> 
> Ouch. If iscsilun->block_size is 4k, this ends up changing
> bs->bl.request_alignment to 512, which is wrong. :(
> 
> If I'm in time, let's drop this from the pull request, and I'll send a
> followup patch.
> 

Fixed myself (s/MIN_NON_ZERO/MAX/) and v2 sent.

Paolo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]