Linux Virtualization - Mar 2013 - [PATCH 0/2] Fix booting tcm_vhost + seabios

Asias He (2):
  virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
  virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}

 src/virtio-scsi.c | 5 +++--
 src/virtio-scsi.h | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

-- 
1.8.1.4

Before we start scsi target scanning, we need to set the
VIRTIO_CONFIG_S_DRIVER_OK flag so the device can do setup properly.

This fix a bug when booting tcm_vhost with seabios.

Signed-off-by: Asias He <asias at redhat.com>
Acked-by: Paolo Bonzini <pbonzini at redhat.com>
---
 src/virtio-scsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c
index 879ddfb..4de1255 100644
--- a/src/virtio-scsi.c
+++ b/src/virtio-scsi.c
@@ -147,6 +147,9 @@ init_virtio_scsi(struct pci_device *pci)
         goto fail;
     }
 
+    vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
+                  VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
+
     int i, tot;
     for (tot = 0, i = 0; i < 256; i++)
         tot += virtio_scsi_scan_target(pci, ioaddr, vq, i);
@@ -154,8 +157,6 @@ init_virtio_scsi(struct pci_device *pci)
     if (!tot)
         goto fail;
 
-    vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
-                  VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
     return;
 
 fail:
-- 
1.8.1.4

Device needs the exact size of these data structure. Prevent padding.

This fixes guest hang when booting seabios + tcm_vhost.

Signed-off-by: Asias He <asias at redhat.com>
---
 src/virtio-scsi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/virtio-scsi.h b/src/virtio-scsi.h
index bbfbf30..96c3701 100644
--- a/src/virtio-scsi.h
+++ b/src/virtio-scsi.h
@@ -26,7 +26,7 @@ struct virtio_scsi_req_cmd {
     u8 prio;
     u8 crn;
     char cdb[VIRTIO_SCSI_CDB_SIZE];
-};
+} __attribute__((packed));
 
 /* This is the first element of the "in" scatter-gather list. */
 struct virtio_scsi_resp_cmd {
@@ -36,7 +36,7 @@ struct virtio_scsi_resp_cmd {
     u8 status;
     u8 response;
     u8 sense[VIRTIO_SCSI_SENSE_SIZE];
-};
+} __attribute__((packed));
 
 #define VIRTIO_SCSI_S_OK            0
 
-- 
1.8.1.4

Il 15/03/2013 02:45, Asias He ha scritto:
> Device needs the exact size of these data structure. Prevent padding.
> 
> This fixes guest hang when booting seabios + tcm_vhost.
> 
> Signed-off-by: Asias He <asias at redhat.com>
> ---
>  src/virtio-scsi.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/virtio-scsi.h b/src/virtio-scsi.h
> index bbfbf30..96c3701 100644
> --- a/src/virtio-scsi.h
> +++ b/src/virtio-scsi.h
> @@ -26,7 +26,7 @@ struct virtio_scsi_req_cmd {
>      u8 prio;
>      u8 crn;
>      char cdb[VIRTIO_SCSI_CDB_SIZE];
> -};
> +} __attribute__((packed));
>  
>  /* This is the first element of the "in" scatter-gather list. */
>  struct virtio_scsi_resp_cmd {
> @@ -36,7 +36,7 @@ struct virtio_scsi_resp_cmd {
>      u8 status;
>      u8 response;
>      u8 sense[VIRTIO_SCSI_SENSE_SIZE];
> -};
> +} __attribute__((packed));
>  
>  #define VIRTIO_SCSI_S_OK            0
>  
> 
Reviewed-by: Paolo Bonzini <pbonzini at redhat.com>

On Fri, Mar 15, 2013 at 09:45:15AM +0800, Asias He
wrote:
> Before we start scsi target scanning, we need to set the
> VIRTIO_CONFIG_S_DRIVER_OK flag so the device can do setup properly.
> 
> This fix a bug when booting tcm_vhost with seabios.
> 
> Signed-off-by: Asias He <asias at redhat.com>
> Acked-by: Paolo Bonzini <pbonzini at redhat.com>
Acked-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  src/virtio-scsi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c
> index 879ddfb..4de1255 100644
> --- a/src/virtio-scsi.c
> +++ b/src/virtio-scsi.c
> @@ -147,6 +147,9 @@ init_virtio_scsi(struct pci_device *pci)
>          goto fail;
>      }
>  
> +    vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> +                  VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
> +
>      int i, tot;
>      for (tot = 0, i = 0; i < 256; i++)
>          tot += virtio_scsi_scan_target(pci, ioaddr, vq, i);
> @@ -154,8 +157,6 @@ init_virtio_scsi(struct pci_device *pci)
>      if (!tot)
>          goto fail;
>  
> -    vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> -                  VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
>      return;
>  
>  fail:
> -- 
> 1.8.1.4

On Fri, Mar 15, 2013 at 09:45:16AM +0800, Asias He
wrote:
> Device needs the exact size of these data structure. Prevent padding.
> 
> This fixes guest hang when booting seabios + tcm_vhost.
> 
> Signed-off-by: Asias He <asias at redhat.com>
Acked-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  src/virtio-scsi.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/virtio-scsi.h b/src/virtio-scsi.h
> index bbfbf30..96c3701 100644
> --- a/src/virtio-scsi.h
> +++ b/src/virtio-scsi.h
> @@ -26,7 +26,7 @@ struct virtio_scsi_req_cmd {
>      u8 prio;
>      u8 crn;
>      char cdb[VIRTIO_SCSI_CDB_SIZE];
> -};
> +} __attribute__((packed));
>  
>  /* This is the first element of the "in" scatter-gather list. */
>  struct virtio_scsi_resp_cmd {
> @@ -36,7 +36,7 @@ struct virtio_scsi_resp_cmd {
>      u8 status;
>      u8 response;
>      u8 sense[VIRTIO_SCSI_SENSE_SIZE];
> -};
> +} __attribute__((packed));
>  
>  #define VIRTIO_SCSI_S_OK            0
>  
> -- 
> 1.8.1.4

On Fri, Mar 15, 2013 at 09:45:14AM +0800, Asias He
wrote:
> Asias He (2):
>   virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
>   virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}
Thanks.  I pushed these patches.

-Kevin

On Fri, 2013-03-15 at 09:45 +0800, Asias He wrote:
> Asias He (2):
>   virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
>   virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}
> 
>  src/virtio-scsi.c | 5 +++--
>  src/virtio-scsi.h | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
Hi Asias,

Thanks for taking the initiative on this, and nice work tracking both of
these stubborn bugs down.

So with these out of the way, we're good to go for an RFC of Paolo's
vhost-scsi-pci code for upstream QEMU, yes..?

I'll have some extra bandwidth this week to spend time on the RFC if
you'd like, otherwise I'm happy with you making the upstream QEMU push
for Paolo's code. 

Whatever works best for you.  :)

Thank you,

--nab