[Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[Paolo Bonzini]

The following changes since commit 76ee152a86d5f2533443ce4d2be6fe253cfb3c45:

  Update version to 1.1.0-rc2 (2012-05-14 17:56:50 -0500)

are available in the git repository at:

  git://github.com/bonzini/qemu.git scsi-next

for you to fetch changes up to e1a2d34f4abd8a117f5c5a25a5bb2e67d597de23:

  ISCSI: call qemu_notify_event() after updating events (2012-05-22 14:14:05 +0200)

----------------------------------------------------------------
Jim Meyering (1):
      scsi: declare vmstate_info_scsi_requests to be static

Ronnie Sahlberg (1):
      ISCSI: call qemu_notify_event() after updating events

 block/iscsi.c |    7 +++++++
 hw/scsi-bus.c |    2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)
-- 
1.7.10.1

[Qemu-devel] [PATCH 1/2] scsi: declare vmstate_info_scsi_requests to be static

[Paolo Bonzini]

From: Jim Meyering <meyering@redhat.com>

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 hw/scsi-bus.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 8ab9bcd..f10f3ec 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1561,7 +1561,7 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-const VMStateInfo vmstate_info_scsi_requests = {
+static const VMStateInfo vmstate_info_scsi_requests = {
     .name = "scsi-requests",
     .get  = get_scsi_requests,
     .put  = put_scsi_requests,
-- 
1.7.10.1

[Qemu-devel] [PATCH 2/2] ISCSI: call qemu_notify_event() after updating events

[Paolo Bonzini]

From: Ronnie Sahlberg <ronniesahlberg@gmail.com>

Otherwise, If we add an event for -is-writeable but the socket is
already writeable there may be a delay before the event callback
is actually triggered.

Those delays would in particular hurt performance during BIOS boot and
when the GRUB bootloader reads the kernel and initrd.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 block/iscsi.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index d37c4ee..f956824 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -109,6 +109,13 @@ iscsi_set_events(IscsiLun *iscsilun)
                            (iscsi_which_events(iscsi) & POLLOUT)
                            ? iscsi_process_write : NULL,
                            iscsi_process_flush, iscsilun);
+
+    /* If we just added the event for writeable we must call
+       and the socket is already writeable the callback might
+       not be invoked until after a short delay unless we call
+       qemu_notify_event().
+     */
+    qemu_notify_event();
 }
 
 static void
-- 
1.7.10.1

Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[Paolo Bonzini]

Il 22/05/2012 14:25, Paolo Bonzini ha scritto:
> The following changes since commit 76ee152a86d5f2533443ce4d2be6fe253cfb3c45:
> 
>   Update version to 1.1.0-rc2 (2012-05-14 17:56:50 -0500)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git scsi-next
> 
> for you to fetch changes up to e1a2d34f4abd8a117f5c5a25a5bb2e67d597de23:
> 
>   ISCSI: call qemu_notify_event() after updating events (2012-05-22 14:14:05 +0200)

Rebased, commit cd0b3cc9eee6f9b6b165f24787b4290bdc1c3f68 is now the tip
of the branch.

Paolo

> ----------------------------------------------------------------
> Jim Meyering (1):
>       scsi: declare vmstate_info_scsi_requests to be static
> 
> Ronnie Sahlberg (1):
>       ISCSI: call qemu_notify_event() after updating events
> 
>  block/iscsi.c |    7 +++++++
>  hw/scsi-bus.c |    2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)

Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[ronnie sahlberg]

Paolo,

You need this patch too since without it it might crash.

commit 6e46eb1846a862dad253be1a576f8554071b154a
Author: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Date:   Sat May 26 10:28:05 2012 +1000

    ISCSI: We can only do the shortcircuit and write directly to the socket
    IFF we know the socket is open (and writeable).
    If the target is more than insignificant distance away, we could otherwise
    try to write to the socket before the nonblocking connect has completed
    which would cause a crash.

    Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>


The other patches in the series can wait until later,  but this one is
needed for 1.1


regards
ronnie sahlberg



On Fri, May 25, 2012 at 9:11 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 22/05/2012 14:25, Paolo Bonzini ha scritto:
>> The following changes since commit 76ee152a86d5f2533443ce4d2be6fe253cfb3c45:
>>
>>   Update version to 1.1.0-rc2 (2012-05-14 17:56:50 -0500)
>>
>> are available in the git repository at:
>>
>>   git://github.com/bonzini/qemu.git scsi-next
>>
>> for you to fetch changes up to e1a2d34f4abd8a117f5c5a25a5bb2e67d597de23:
>>
>>   ISCSI: call qemu_notify_event() after updating events (2012-05-22 14:14:05 +0200)
>
> Rebased, commit cd0b3cc9eee6f9b6b165f24787b4290bdc1c3f68 is now the tip
> of the branch.
>
> Paolo
>
>> ----------------------------------------------------------------
>> Jim Meyering (1):
>>       scsi: declare vmstate_info_scsi_requests to be static
>>
>> Ronnie Sahlberg (1):
>>       ISCSI: call qemu_notify_event() after updating events
>>
>>  block/iscsi.c |    7 +++++++
>>  hw/scsi-bus.c |    2 +-
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>
>

Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[Paolo Bonzini]

Il 26/05/2012 07:41, ronnie sahlberg ha scritto:
> Paolo,
> 
> You need this patch too since without it it might crash.
> 
> commit 6e46eb1846a862dad253be1a576f8554071b154a
> Author: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Date:   Sat May 26 10:28:05 2012 +1000
> 
>     ISCSI: We can only do the shortcircuit and write directly to the socket
>     IFF we know the socket is open (and writeable).
>     If the target is more than insignificant distance away, we could otherwise
>     try to write to the socket before the nonblocking connect has completed
>     which would cause a crash.
> 
>     Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> 
> 
> The other patches in the series can wait until later,  but this one is
> needed for 1.1

I didn't get the patch, so I redid it.

Can you test asap git://github.com/bonzini/qemu.git, branch
scsi-candidate-1.1 to see if it matches what you want to be in 1.1?

Paolo

Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[ronnie sahlberg]

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

Almost,
but connecter=1 should be set after the if statement not inside it.

     if (status != 0) {
+        itask->iscsilun->connected = 1;
         itask->status   = 1;

I.e. dont set it in the error path, set it after the block. then it
should be all good.



optionally you can revert 5b5e96bba6835b794ba237c1ddd6580dea8d4aef
and use the patch I attach instead


regards
ronnie sahlberg


 2012 at 5:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 26/05/2012 07:41, ronnie sahlberg ha scritto:
>> Paolo,
>>
>> You need this patch too since without it it might crash.
>>
>> commit 6e46eb1846a862dad253be1a576f8554071b154a
>> Author: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>> Date:   Sat May 26 10:28:05 2012 +1000
>>
>>     ISCSI: We can only do the shortcircuit and write directly to the socket
>>     IFF we know the socket is open (and writeable).
>>     If the target is more than insignificant distance away, we could otherwise
>>     try to write to the socket before the nonblocking connect has completed
>>     which would cause a crash.
>>
>>     Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>
>>
>> The other patches in the series can wait until later,  but this one is
>> needed for 1.1
>
> I didn't get the patch, so I redid it.
>
> Can you test asap git://github.com/bonzini/qemu.git, branch
> scsi-candidate-1.1 to see if it matches what you want to be in 1.1?
>
> Paolo

[-- Attachment #2: 0001-ISCSI-We-can-only-do-the-shortcircuit-and-write-dire.patch --]
[-- Type: application/octet-stream, Size: 1359 bytes --]

From 6e46eb1846a862dad253be1a576f8554071b154a Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Date: Sat, 26 May 2012 10:28:05 +1000
Subject: [PATCH 1/5] ISCSI: We can only do the shortcircuit and write directly to the socket
 IFF we know the socket is open (and writeable).
 If the target is more than insignificant distance away, we could otherwise
 try to write to the socket before the nonblocking connect has completed
 which would cause a crash.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 block/iscsi.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 0d40637..df0b6c8 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -117,10 +117,14 @@ iscsi_set_events(IscsiLun *iscsilun)
     int ev;
 
     /* Try to write as much as we can to the socket
-     * without setting up an event
+     * without setting up an event.
+     * Only do this if we are completely logged in, so we know that
+     * the socket is in connected state.
      */
-    if (iscsi_which_events(iscsi) & POLLOUT) {
-        iscsi_process_write(iscsilun);
+    if (iscsi_is_logged_in(iscsi)) {
+        if (iscsi_which_events(iscsi) & POLLOUT) {
+            iscsi_process_write(iscsilun);
+        }
     }
 
     /* We always register a read handler.  */
-- 
1.7.3.1

Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[ronnie sahlberg]

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

Im compiling your branch now and will verify all is good.

It is missing an include  so you need to add this patch to compile :




On Sat, May 26, 2012 at 5:59 PM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> Almost,
> but connecter=1 should be set after the if statement not inside it.
>
>     if (status != 0) {
> +        itask->iscsilun->connected = 1;
>         itask->status   = 1;
>
> I.e. dont set it in the error path, set it after the block. then it
> should be all good.
>
>
>
> optionally you can revert 5b5e96bba6835b794ba237c1ddd6580dea8d4aef
> and use the patch I attach instead
>
>
> regards
> ronnie sahlberg
>
>
>  2012 at 5:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 26/05/2012 07:41, ronnie sahlberg ha scritto:
>>> Paolo,
>>>
>>> You need this patch too since without it it might crash.
>>>
>>> commit 6e46eb1846a862dad253be1a576f8554071b154a
>>> Author: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>> Date:   Sat May 26 10:28:05 2012 +1000
>>>
>>>     ISCSI: We can only do the shortcircuit and write directly to the socket
>>>     IFF we know the socket is open (and writeable).
>>>     If the target is more than insignificant distance away, we could otherwise
>>>     try to write to the socket before the nonblocking connect has completed
>>>     which would cause a crash.
>>>
>>>     Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>>
>>>
>>> The other patches in the series can wait until later,  but this one is
>>> needed for 1.1
>>
>> I didn't get the patch, so I redid it.
>>
>> Can you test asap git://github.com/bonzini/qemu.git, branch
>> scsi-candidate-1.1 to see if it matches what you want to be in 1.1?
>>
>> Paolo

[-- Attachment #2: 0001-ISCSI-need-to-include-scsi-defs.h.patch --]
[-- Type: application/octet-stream, Size: 592 bytes --]

From 781262c389cb0b77e7aab4639e0a51e4bc115029 Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Date: Sat, 26 May 2012 18:02:27 +1000
Subject: [PATCH] ISCSI  need to include scsi-defs.h

---
 block/iscsi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index eb16aa9..76427e8 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -30,6 +30,7 @@
 #include "qemu-error.h"
 #include "block_int.h"
 #include "trace.h"
+#include "hw/scsi-defs.h"
 
 #include <iscsi/iscsi.h>
 #include <iscsi/scsi-lowlevel.h>
-- 
1.7.3.1

Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[ronnie sahlberg]

I have compiled your branch and run through some tests.

It all looks good as long as you apply the patch to #include "hw/scsi-defs.h"



On Sat, May 26, 2012 at 6:17 PM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> Im compiling your branch now and will verify all is good.
>
> It is missing an include  so you need to add this patch to compile :
>
>
>
>
> On Sat, May 26, 2012 at 5:59 PM, ronnie sahlberg
> <ronniesahlberg@gmail.com> wrote:
>> Almost,
>> but connecter=1 should be set after the if statement not inside it.
>>
>>     if (status != 0) {
>> +        itask->iscsilun->connected = 1;
>>         itask->status   = 1;
>>
>> I.e. dont set it in the error path, set it after the block. then it
>> should be all good.
>>
>>
>>
>> optionally you can revert 5b5e96bba6835b794ba237c1ddd6580dea8d4aef
>> and use the patch I attach instead
>>
>>
>> regards
>> ronnie sahlberg
>>
>>
>>  2012 at 5:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 26/05/2012 07:41, ronnie sahlberg ha scritto:
>>>> Paolo,
>>>>
>>>> You need this patch too since without it it might crash.
>>>>
>>>> commit 6e46eb1846a862dad253be1a576f8554071b154a
>>>> Author: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>>> Date:   Sat May 26 10:28:05 2012 +1000
>>>>
>>>>     ISCSI: We can only do the shortcircuit and write directly to the socket
>>>>     IFF we know the socket is open (and writeable).
>>>>     If the target is more than insignificant distance away, we could otherwise
>>>>     try to write to the socket before the nonblocking connect has completed
>>>>     which would cause a crash.
>>>>
>>>>     Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>>>
>>>>
>>>> The other patches in the series can wait until later,  but this one is
>>>> needed for 1.1
>>>
>>> I didn't get the patch, so I redid it.
>>>
>>> Can you test asap git://github.com/bonzini/qemu.git, branch
>>> scsi-candidate-1.1 to see if it matches what you want to be in 1.1?
>>>
>>> Paolo

Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

[Paolo Bonzini]

Il 26/05/2012 11:18, ronnie sahlberg ha scritto:
> I have compiled your branch and run through some tests.
> 
> It all looks good as long as you apply the patch to #include "hw/scsi-defs.h"

Thanks, I updated the scsi-next branch.

Paolo