From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Td0o0-000064-MC for qemu-devel@nongnu.org; Mon, 26 Nov 2012 10:41:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Td0nu-0007hQ-Hn for qemu-devel@nongnu.org; Mon, 26 Nov 2012 10:41:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37002) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Td0nu-0007hK-AR for qemu-devel@nongnu.org; Mon, 26 Nov 2012 10:41:02 -0500 Date: Mon, 26 Nov 2012 16:40:54 +0100 From: Stefan Hajnoczi Message-ID: <20121126154054.GA25839@stefanha-thinkpad.redhat.com> References: <1353595852-30776-1-git-send-email-fred.konrad@greensocs.com> <1353595852-30776-2-git-send-email-fred.konrad@greensocs.com> <87k3t8qvrg.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87k3t8qvrg.fsf@codemonkey.ws> Subject: Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: peter.maydell@linaro.org, e.voevodin@samsung.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, cornelia.huck@de.ibm.com, afaerber@suse.de, fred.konrad@greensocs.com On Mon, Nov 26, 2012 at 08:33:23AM -0600, Anthony Liguori wrote: > fred.konrad@greensocs.com writes: > > > From: KONRAD Frederic > > > > This patch create a new VirtioBus, which can be added to Virtio transports like > > virtio-pci, virtio-mmio,... > > > > One VirtIODevice can be connected to this device, like virtio-blk in the 3rd > > patch. > > > > The VirtioBus shares through a VirtioBusInfo structure : > > > > * two callbacks with the transport : init_cb and exit_cb, which must be > > called by the VirtIODevice, after the initialization and before the > > destruction, to put the right PCI IDs and/or stop the event fd. > > > > * a VirtIOBindings structure. > > > > Signed-off-by: KONRAD Frederic > > --- > > hw/Makefile.objs | 1 + > > hw/virtio-bus.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > hw/virtio-bus.h | 58 ++++++++++++++++++++++ > > 3 files changed, 207 insertions(+) > > create mode 100644 hw/virtio-bus.c > > create mode 100644 hw/virtio-bus.h > > > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > > index ea46f81..bd14d1b 100644 > > --- a/hw/Makefile.objs > > +++ b/hw/Makefile.objs > > @@ -2,6 +2,7 @@ common-obj-y = usb/ ide/ > > common-obj-y += loader.o > > common-obj-$(CONFIG_VIRTIO) += virtio-console.o > > common-obj-$(CONFIG_VIRTIO) += virtio-rng.o > > +common-obj-$(CONFIG_VIRTIO) += virtio-bus.o > > common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o > > common-obj-y += fw_cfg.o > > common-obj-$(CONFIG_PCI) += pci.o pci_bridge.o pci_bridge_dev.o > > diff --git a/hw/virtio-bus.c b/hw/virtio-bus.c > > new file mode 100644 > > index 0000000..991b6f5 > > --- /dev/null > > +++ b/hw/virtio-bus.c > > @@ -0,0 +1,148 @@ > > +/* > > + * VirtioBus > > + * > > + * Copyright (C) 2012 : GreenSocs Ltd > > + * http://www.greensocs.com/ , email: info@greensocs.com > > + * > > + * Developed by : > > + * Frederic Konrad > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation, either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License along > > + * with this program; if not, see . > > + * > > + */ > > + > > +#include "hw.h" > > +#include "qemu-error.h" > > +#include "qdev.h" > > +#include "virtio-bus.h" > > +#include "virtio.h" > > + > > +#define DEBUG_VIRTIO_BUS 1 > > + > > +#define DPRINTF(fmt, ...) if (DEBUG_VIRTIO_BUS) { \ > > + printf("virtio_bus: " fmt , ## __VA_ARGS__); \ > > + } > > #ifdef DEBUG_VIRTIO_BUS > #define DPRINTF(fmt, ...) ... > #else > #define DPRINTF(fmt, ...) do { } while (0) > #endif > > You're leaving a dangling if clause which can do very strange things if > used before an else statement. This should be: #define DEBUG_VIRTIO_BUS 1 #define DPRINTF(fmt, ...) \ do { \ if (DEBUG_VIRTIO_BUS) { \ printf("virtio_bus: " fmt , ## __VA_ARGS__); \ } \ } while (0) This way there's no dangling else statement problem. But it also ensures the we always compile the debug print, thereby avoiding bitrot you get from #ifdef ... #else ... #endif. Stefan