From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36775) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIep9-0001XD-HQ for qemu-devel@nongnu.org; Thu, 21 Mar 2013 08:42:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIep7-0002eH-NZ for qemu-devel@nongnu.org; Thu, 21 Mar 2013 08:42:27 -0400 Received: from greensocs.com ([87.106.252.221]:38413 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIep7-0002dl-Cx for qemu-devel@nongnu.org; Thu, 21 Mar 2013 08:42:25 -0400 Message-ID: <514B002B.4030403@greensocs.com> Date: Thu, 21 Mar 2013 13:42:19 +0100 From: =?ISO-8859-1?Q?KONRAD_Fr=E9d=E9ric?= MIME-Version: 1.0 References: <1363788463-27462-1-git-send-email-fred.konrad@greensocs.com> <1363788463-27462-4-git-send-email-fred.konrad@greensocs.com> <20130321131001.31361195@gondolin> In-Reply-To: <20130321131001.31361195@gondolin> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 03/10] virtio-scsi: moving host_features from properties to transport properties. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, "Paolo Bonzini (supporter:SCSI)" On 21/03/2013 13:10, Cornelia Huck wrote: > On Wed, 20 Mar 2013 15:07:36 +0100 > fred.konrad@greensocs.com wrote: > >> From: KONRAD Frederic >> >> host_features field is part of the transport device. So move all the >> host_features related properties into transport device. >> >> Signed-off-by: KONRAD Frederic >> --- >> hw/s390x/s390-virtio-bus.c | 7 ++++++- >> hw/s390x/virtio-ccw.c | 7 ++++++- >> hw/virtio-pci.c | 7 ++++++- >> hw/virtio-scsi.h | 9 +++------ >> 4 files changed, 21 insertions(+), 9 deletions(-) >> >> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c >> index 76bc99a..265d94f 100644 >> --- a/hw/s390x/s390-virtio-bus.c >> +++ b/hw/s390x/s390-virtio-bus.c >> @@ -543,7 +543,12 @@ static const TypeInfo virtio_s390_device_info = { >> }; >> >> static Property s390_virtio_scsi_properties[] = { >> - DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOS390Device, host_features, scsi), >> + DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOS390Device, scsi), >> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features), >> + DEFINE_PROP_BIT("hotplug", VirtIOS390Device, host_features, >> + VIRTIO_SCSI_F_HOTPLUG, true), >> + DEFINE_PROP_BIT("param_change", VirtIOS390Device, host_features, >> + VIRTIO_SCSI_F_CHANGE, true), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >> index 9688835..71a51d9 100644 >> --- a/hw/s390x/virtio-ccw.c >> +++ b/hw/s390x/virtio-ccw.c >> @@ -836,7 +836,12 @@ static const TypeInfo virtio_ccw_balloon = { >> >> static Property virtio_ccw_scsi_properties[] = { >> DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), >> - DEFINE_VIRTIO_SCSI_PROPERTIES(VirtioCcwDevice, host_features[0], scsi), >> + DEFINE_VIRTIO_SCSI_PROPERTIES(VirtioCcwDevice, scsi), >> + DEFINE_VIRTIO_COMMON_FEATURES(VirtioCcwDevice, host_features[0]), >> + DEFINE_PROP_BIT("hotplug", VirtioCcwDevice, host_features[0], >> + VIRTIO_SCSI_F_HOTPLUG, true), >> + DEFINE_PROP_BIT("param_change", VirtioCcwDevice, host_features[0], >> + VIRTIO_SCSI_F_CHANGE, true), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >> index f3ece78..0665b04 100644 >> --- a/hw/virtio-pci.c >> +++ b/hw/virtio-pci.c >> @@ -1221,7 +1221,12 @@ static void virtio_scsi_exit_pci(PCIDevice *pci_dev) >> static Property virtio_scsi_properties[] = { >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), >> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), >> - DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, host_features, scsi), >> + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), >> + DEFINE_PROP_BIT("hotplug", VirtIOPCIProxy, host_features, >> + VIRTIO_SCSI_F_HOTPLUG, true), >> + DEFINE_PROP_BIT("param_change", VirtIOPCIProxy, host_features, >> + VIRTIO_SCSI_F_CHANGE, true), >> + DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, scsi), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h >> index fb83b67..536c4c3 100644 >> --- a/hw/virtio-scsi.h >> +++ b/hw/virtio-scsi.h >> @@ -47,12 +47,9 @@ typedef struct VirtIOSCSI { >> VirtQueue **cmd_vqs; >> } VirtIOSCSI; >> >> -#define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _features_field, _conf_field) \ >> - DEFINE_VIRTIO_COMMON_FEATURES(_state, _features_field), \ >> +#define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field) \ >> DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \ >> - DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF), \ >> - DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128), \ >> - DEFINE_PROP_BIT("hotplug", _state, _features_field, VIRTIO_SCSI_F_HOTPLUG, true), \ >> - DEFINE_PROP_BIT("param_change", _state, _features_field, VIRTIO_SCSI_F_CHANGE, true) >> + DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF),\ >> + DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128) > Would it make sense to add > > #define DEFINE_VIRTIO_SCSI_FEATURES(_state, _features_field) \ > DEFINE_VIRTIO_COMMON_FEATURES(_state, _features_field), \ > DEFINE_PROP_BIT("hotplug", _state, _features_field, VIRTIO_SCSI_F_HOTPLUG, true), \ > DEFINE_PROP_BIT("param_change", _state, _features_field, VIRTIO_SCSI_F_CHANGE, true) > > to avoid code duplication? Sure, so I'll put it in virtio-blk.h. > >> #endif /* _QEMU_VIRTIO_SCSI_H */