Re: [Qemu-devel] [PATCH RFC 08/11] scsi: SCSIDiskReq declaration moved to header

[Hannes Reinecke <hare@suse.de>]

On 08/18/2015 01:42 AM, Alexander Bezzubikov wrote:
> Signed-off-by: Alexander Bezzubikov <abezzubikov@ispras.ru>
> ---
>  hw/scsi/scsi-disk.c    | 12 ------------
>  include/hw/scsi/scsi.h | 13 +++++++++++++
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index f67d816..9d5f0a4 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -53,18 +53,6 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
>  
>  typedef struct SCSIDiskState SCSIDiskState;
>  
> -typedef struct SCSIDiskReq {
> -    SCSIRequest req;
> -    /* Both sector and sector_count are in terms of qemu 512 byte blocks.  */
> -    uint64_t sector;
> -    uint32_t sector_count;
> -    uint32_t buflen;
> -    bool started;
> -    struct iovec iov;
> -    QEMUIOVector qiov;
> -    BlockAcctCookie acct;
> -} SCSIDiskReq;
> -
>  #define SCSI_DISK_F_REMOVABLE             0
>  #define SCSI_DISK_F_DPOFUA                1
>  #define SCSI_DISK_F_NO_REMOVABLE_DEVOPS   2
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index 881ed23..de0546e 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -6,6 +6,7 @@
>  #include "hw/block/block.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/notify.h"
> +#include "block/accounting.h"
>  
>  #define MAX_SCSI_DEVS	255
>  
> @@ -120,6 +121,18 @@ extern const VMStateDescription vmstate_scsi_device;
>      .offset     = vmstate_offset_value(_state, _field, SCSIDevice),  \
>  }
>  
> +typedef struct SCSIDiskReq {
> +    SCSIRequest req;
> +    /* Both sector and sector_count are in terms of qemu 512 byte blocks.  */
> +    uint64_t sector;
> +    uint32_t sector_count;
> +    uint32_t buflen;
> +    bool started;
> +    struct iovec iov;
> +    QEMUIOVector qiov;
> +    BlockAcctCookie acct;
> +} SCSIDiskReq;
> +
>  /* cdrom.c */
>  int cdrom_read_toc(int nb_sectors, uint8_t *buf, int msf, int start_track);
>  int cdrom_read_toc_raw(int nb_sectors, uint8_t *buf, int msf, int session_num);
> 
I'm not sure this is a good idea; the abstraction model seems to be that
scsi-disk.h contains all declarations for the SCSI disk model, and as
such the struct should remain there.

Probably it might be better to create a 'scsi-disk.h' include file, to
contain all necessary things.

But actually I'm not sure.
John? What's your opinion here?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)