From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZU29O-0000rx-VL for qemu-devel@nongnu.org; Mon, 24 Aug 2015 20:31:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZU29K-0001L2-VS for qemu-devel@nongnu.org; Mon, 24 Aug 2015 20:31:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45243) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZU29K-0001Kg-QC for qemu-devel@nongnu.org; Mon, 24 Aug 2015 20:31:38 -0400 References: <1439854945-5597-1-git-send-email-abezzubikov@ispras.ru> <1439854945-5597-9-git-send-email-abezzubikov@ispras.ru> <55D2CE03.7050602@suse.de> From: John Snow Message-ID: <55DBB768.2060203@redhat.com> Date: Mon, 24 Aug 2015 20:31:36 -0400 MIME-Version: 1.0 In-Reply-To: <55D2CE03.7050602@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC 08/11] scsi: SCSIDiskReq declaration moved to header List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke , Alexander Bezzubikov , qemu-devel@nongnu.org Cc: abezzubikov@ispras.ru On 08/18/2015 02:17 AM, Hannes Reinecke wrote: > On 08/18/2015 01:42 AM, Alexander Bezzubikov wrote: >> Signed-off-by: Alexander Bezzubikov >> --- >> 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 > I know I'm working from outdated copies here, but my guess would be to attempt to limit exposure of this struct. Putting it in scsi.h probably exposes it to more files than we anticipate. I'll see what happens in v2 through v4 :) --js