From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58402) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFQKn-0006vT-OJ for qemu-devel@nongnu.org; Tue, 12 Mar 2013 10:37:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UFQKi-0000rS-IE for qemu-devel@nongnu.org; Tue, 12 Mar 2013 10:37:45 -0400 Received: from greensocs.com ([87.106.252.221]:57435 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFQKi-0000rN-78 for qemu-devel@nongnu.org; Tue, 12 Mar 2013 10:37:40 -0400 Message-ID: <513F3DAF.8050405@greensocs.com> Date: Tue, 12 Mar 2013 15:37:35 +0100 From: =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= MIME-Version: 1.0 References: <1363080131-16427-1-git-send-email-fred.konrad@greensocs.com> <1363080131-16427-3-git-send-email-fred.konrad@greensocs.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Kevin Wolf , aliguori@us.ibm.com, mst@redhat.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, Stefan Hajnoczi , afaerber@suse.de On 12/03/2013 15:28, Peter Maydell wrote: > On 12 March 2013 09:22, wrote: >> /* The ID for virtio_block */ >> @@ -130,4 +134,28 @@ typedef struct VirtIOBlock { >> #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ >> DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) >> >> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE >> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field) \ >> + DEFINE_PROP_BIT("x-data-plane", _state, _field.data_plane, 0, false), >> +#else >> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field) >> +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */ >> + >> +#ifdef __linux__ >> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field) \ >> + DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true), >> +#else >> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field) >> +#endif /* __linux__ */ >> + >> +#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field) \ >> + DEFINE_BLOCK_PROPERTIES(_state, _field.conf), \ >> + DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf), \ >> + DEFINE_PROP_STRING("serial", _state, _field.serial), \ >> + DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true), \ >> + DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field) \ >> + DEFINE_DATA_PLANE_PROPERTIES(_state, _field) >> + >> +void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); >> + >> #endif >> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >> index 39c1966..9ed0228 100644 >> --- a/hw/virtio-pci.c >> +++ b/hw/virtio-pci.c >> @@ -1084,19 +1084,10 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev) >> >> static Property virtio_blk_properties[] = { >> DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), >> - DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf), >> - DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf), >> - DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial), >> -#ifdef __linux__ >> - DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true), >> -#endif >> - DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0, true), >> DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), >> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE >> - DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false), >> -#endif >> DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), >> DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features), >> + DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk) >> DEFINE_PROP_END_OF_LIST(), > You need to tweak your macro definitions so that the user can > put a comma after DEFINE_VIRTIO_BLK_PROPERTIES() [compare > DEFINE_BLOCK_PROPERTIES and DEFINE_BLOCK_CHS_PROPERTIES]. > Otherwise it's going to get confusing and somebody's going > to add one without noticing that that gets you an extra > empty properties array element. Do you have any idea for that? It's a little tricky with the two conditions CONFIG_VIRTIO_BLK_DATA_PLANE and __linux__, I can't have #define with an #ifdef inside.. Do you see what I mean? > >> }; >> >> -- >> 1.7.11.7 >> > -- PMM