From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40843) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFQoe-0004Ly-7p for qemu-devel@nongnu.org; Tue, 12 Mar 2013 11:08:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UFQoY-0003nR-MU for qemu-devel@nongnu.org; Tue, 12 Mar 2013 11:08:36 -0400 Received: from greensocs.com ([87.106.252.221]:37867 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFQoY-0003nD-Cj for qemu-devel@nongnu.org; Tue, 12 Mar 2013 11:08:30 -0400 Message-ID: <513F44E7.2020306@greensocs.com> Date: Tue, 12 Mar 2013 16:08:23 +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> <513F3DAF.8050405@greensocs.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable 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:42, Peter Maydell wrote: > On 12 March 2013 14:37, KONRAD Fr=C3=A9d=C3=A9ric wrote: >> 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[] =3D { >>>> 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_PL= ANE >> and __linux__, >> >> I can't have #define with an #ifdef inside.. >> >> Do you see what I mean? > Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY > and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not > ones that are expected to be used by other code, right? So you can > define them with commas (and name them something so it's obvious > they're not intended for wider use as property array elements), > and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES > doesn't end with a comma. (You can do that by putting the macros > that expand to maybe-comma-or-not at the front, not the end.) > > -- PMM ok, I can put a comment which say not to use them? Thanks, Fred