From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LBws6-0007YX-2H for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:43:22 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LBws3-0007WW-D6 for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:43:20 -0500 Received: from [199.232.76.173] (port=44174 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LBws3-0007WH-8D for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:43:19 -0500 Received: from mx2.redhat.com ([66.187.237.31]:40450) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LBws0-0002Cu-Ti for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:43:17 -0500 Date: Sun, 14 Dec 2008 21:44:03 +0200 From: Gleb Natapov Message-ID: <20081214194403.GB7215@redhat.com> References: <20081214115027.4028.56164.stgit@dhcp-1-237.tlv.redhat.com> <49455D51.5080004@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <49455D51.5080004@codemonkey.ws> Subject: [Qemu-devel] Re: [PATCH] Vmchannel PCI device. Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org On Sun, Dec 14, 2008 at 01:24:01PM -0600, Anthony Liguori wrote: > Gleb Natapov wrote: >> diff --git a/hw/pc.c b/hw/pc.c >> index 73dd8bc..57e3b1d 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -1095,7 +1095,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size, >> } >> } >> - /* Add virtio block devices */ >> + /* Add virtio devices */ >> > > Please don't make comments less specific. We probably want to go in the > opposite direction :-) > I change the comment because I also changes the code it describes. Previously it registered only block device, now it registers balloon and vmchannel. >> + } >> } >> static void pc_init_pci(ram_addr_t ram_size, int vga_ram_size, >> diff --git a/hw/virtio-vmchannel.c b/hw/virtio-vmchannel.c >> new file mode 100644 >> index 0000000..1f5e274 >> --- /dev/null >> +++ b/hw/virtio-vmchannel.c >> @@ -0,0 +1,283 @@ >> +/* >> + * Virtio VMChannel Device >> + * >> + * Copyright RedHat, inc. 2008 >> + * >> + * Authors: >> + * Gleb Natapov >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2. See >> + * the COPYING file in the top-level directory. >> > > Did you intend for GPLv2 or GPLv2+? There's no requirement either way > but sometimes people just copy/paste these things. > Will check :) >> + */ >> + >> +#include "qemu-common.h" >> +#include "sysemu.h" >> +#include "virtio.h" >> +#include "pc.h" >> +#include "qemu-char.h" >> +#include "virtio-vmchannel.h" >> + >> +//#define DEBUG_VMCHANNEL >> + >> +#ifdef DEBUG_VMCHANNEL >> +#define VMCHANNEL_DPRINTF(fmt, args...) \ >> + do { printf("VMCHANNEL: " fmt , ##args); } while (0) >> +#else >> +#define VMCHANNEL_DPRINTF(fmt, args...) >> +#endif >> > > I very much like just naming these things dprintf() but this is not a > requirement. Please do use C99 style though instead of GCC-ism. > OK. >> diff --git a/sysemu.h b/sysemu.h >> index 94cffaf..54d9c83 100644 >> --- a/sysemu.h >> +++ b/sysemu.h >> @@ -157,6 +157,10 @@ extern CharDriverState *parallel_hds[MAX_PARALLEL_PORTS]; >> #define TFR(expr) do { if ((expr) != -1) break; } while (errno == >> EINTR) >> +#define MAX_VMCHANNEL_DEVICES 4 >> +void virtio_vmchannel_init(PCIBus *bus); >> +void vmchannel_init(CharDriverState *hd, const char *name); >> > > This should be in virtio-vmchannel.h. > OK. >> - case QEMU_OPTION_loadvm: >> + case QEMU_OPTION_vmchannel: >> + if (vmchannel_device_index >= MAX_VMCHANNEL_DEVICES) { >> + fprintf(stderr, "qemu: too many vmchannel devices\n"); >> + exit(1); >> + } >> + vmchannel_devices[vmchannel_device_index++] = strdup(optarg); >> > > No need to strdup(). optarg is good for the duration of execution. > optarg is const and I change the string during parsing (call strsep on it). -- Gleb.