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 <[email protected]> >> + * >> + * 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
