| Message ID | 874o5kx9uk.fsf@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 2011-04-27 12:33, Aneesh Kumar K.V wrote: > On Wed, 27 Apr 2011 09:03:56 +0200, Jan Kiszka <jan.kiszka@web.de> wrote: >> On 2011-04-27 08:53, Aneesh Kumar K.V wrote: >>> Now that we start adding more files related to 9pfs >>> it make sense to move them to a separate directory >>> >>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> >>> --- >>> Makefile.objs | 10 +++++++--- >>> Makefile.target | 6 ++++-- >>> configure | 2 ++ >>> {hw => fsdev}/file-op-9p.h | 0 >>> fsdev/qemu-fsdev.h | 2 +- >>> hw/{ => 9pfs}/virtio-9p-debug.c | 0 >>> hw/{ => 9pfs}/virtio-9p-debug.h | 0 >>> hw/{ => 9pfs}/virtio-9p-local.c | 0 >>> hw/{ => 9pfs}/virtio-9p-posix-acl.c | 2 +- >>> hw/{ => 9pfs}/virtio-9p-xattr-user.c | 2 +- >>> hw/{ => 9pfs}/virtio-9p-xattr.c | 2 +- >>> hw/{ => 9pfs}/virtio-9p-xattr.h | 0 >>> hw/{ => 9pfs}/virtio-9p.c | 0 >>> hw/{ => 9pfs}/virtio-9p.h | 2 +- >>> 14 files changed, 18 insertions(+), 10 deletions(-) >>> rename {hw => fsdev}/file-op-9p.h (100%) >>> rename hw/{ => 9pfs}/virtio-9p-debug.c (100%) >>> rename hw/{ => 9pfs}/virtio-9p-debug.h (100%) >>> rename hw/{ => 9pfs}/virtio-9p-local.c (100%) >>> rename hw/{ => 9pfs}/virtio-9p-posix-acl.c (99%) >>> rename hw/{ => 9pfs}/virtio-9p-xattr-user.c (98%) >>> rename hw/{ => 9pfs}/virtio-9p-xattr.c (99%) >>> rename hw/{ => 9pfs}/virtio-9p-xattr.h (100%) >>> rename hw/{ => 9pfs}/virtio-9p.c (100%) >> >> That's a good chance to split up this file, move virtio_9p_get_config >> into a separate one and build the large virtio-9p.c as part of hwlib >> while keeping the new file target-specific. I've some hack for this >> lying around, but now that you are already at it... >> > > How about doing the below patch also and move all those device specific > stuff to virtio-9p-device.c and rest in virtio-9p.c ? Looks ok. Removing #ifdefs is generally welcome. Jan
On 04/27/2011 03:33 AM, Aneesh Kumar K.V wrote: > On Wed, 27 Apr 2011 09:03:56 +0200, Jan Kiszka<jan.kiszka@web.de> wrote: >> On 2011-04-27 08:53, Aneesh Kumar K.V wrote: >>> Now that we start adding more files related to 9pfs >>> it make sense to move them to a separate directory >>> >>> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> >>> --- >>> Makefile.objs | 10 +++++++--- >>> Makefile.target | 6 ++++-- >>> configure | 2 ++ >>> {hw => fsdev}/file-op-9p.h | 0 >>> fsdev/qemu-fsdev.h | 2 +- >>> hw/{ => 9pfs}/virtio-9p-debug.c | 0 >>> hw/{ => 9pfs}/virtio-9p-debug.h | 0 >>> hw/{ => 9pfs}/virtio-9p-local.c | 0 >>> hw/{ => 9pfs}/virtio-9p-posix-acl.c | 2 +- >>> hw/{ => 9pfs}/virtio-9p-xattr-user.c | 2 +- >>> hw/{ => 9pfs}/virtio-9p-xattr.c | 2 +- >>> hw/{ => 9pfs}/virtio-9p-xattr.h | 0 >>> hw/{ => 9pfs}/virtio-9p.c | 0 >>> hw/{ => 9pfs}/virtio-9p.h | 2 +- >>> 14 files changed, 18 insertions(+), 10 deletions(-) >>> rename {hw => fsdev}/file-op-9p.h (100%) >>> rename hw/{ => 9pfs}/virtio-9p-debug.c (100%) >>> rename hw/{ => 9pfs}/virtio-9p-debug.h (100%) >>> rename hw/{ => 9pfs}/virtio-9p-local.c (100%) >>> rename hw/{ => 9pfs}/virtio-9p-posix-acl.c (99%) >>> rename hw/{ => 9pfs}/virtio-9p-xattr-user.c (98%) >>> rename hw/{ => 9pfs}/virtio-9p-xattr.c (99%) >>> rename hw/{ => 9pfs}/virtio-9p-xattr.h (100%) >>> rename hw/{ => 9pfs}/virtio-9p.c (100%) >> That's a good chance to split up this file, move virtio_9p_get_config >> into a separate one and build the large virtio-9p.c as part of hwlib >> while keeping the new file target-specific. I've some hack for this >> lying around, but now that you are already at it... >> > How about doing the below patch also and move all those device specific > stuff to virtio-9p-device.c and rest in virtio-9p.c ? I think this goes on top of your old patch. I am ready to give pull to Anthony with old patch. May be this can go in next pull? Thanks, JV > commit 9de1857114dac1dcd0c6399d91036c279373b2d0 > Author: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> > Date: Thu Oct 21 13:50:05 2010 +0530 > > virtio-9p: Move 9p device registration into virtio-9p.c > > This patch move the 9p device registration into its own file > > Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> > > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c > index daade77..11e87a3 100644 > --- a/hw/virtio-9p.c > +++ b/hw/virtio-9p.c > @@ -14,6 +14,7 @@ > #include "virtio.h" > #include "pc.h" > #include "qemu_socket.h" > +#include "virtio-pci.h" > #include "virtio-9p.h" > #include "fsdev/qemu-fsdev.h" > #include "virtio-9p-debug.h" > @@ -3741,3 +3742,37 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf) > > return&s->vdev; > } > + > +static int virtio_9p_init_pci(PCIDevice *pci_dev) > +{ > + VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > + VirtIODevice *vdev; > + > + vdev = virtio_9p_init(&pci_dev->qdev,&proxy->fsconf); > + virtio_init_pci(proxy, vdev, > + PCI_VENDOR_ID_REDHAT_QUMRANET, > + 0x1009, > + 0x2, > + 0x00); > + > + return 0; > +} > + > +static PCIDeviceInfo virtio_9p_info = { > + .qdev.name = "virtio-9p-pci", > + .qdev.size = sizeof(VirtIOPCIProxy), > + .init = virtio_9p_init_pci, > + .qdev.props = (Property[]) { > + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > + DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag), > + DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id), > + DEFINE_PROP_END_OF_LIST(), > + } > +}; > + > +static void virtio_9p_register_devices(void) > +{ > + pci_qdev_register(&virtio_9p_info); > +} > + > +device_init(virtio_9p_register_devices) > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 729917d..f1377b1 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -25,6 +25,7 @@ > #include "loader.h" > #include "kvm.h" > #include "blockdev.h" > +#include "virtio-pci.h" > > /* from Linux's linux/virtio_pci.h */ > > @@ -90,26 +91,6 @@ > */ > #define wmb() do { } while (0) > > -/* PCI bindings. */ > - > -typedef struct { > - PCIDevice pci_dev; > - VirtIODevice *vdev; > - uint32_t bugs; > - uint32_t addr; > - uint32_t class_code; > - uint32_t nvectors; > - BlockConf block; > - NICConf nic; > - uint32_t host_features; > -#ifdef CONFIG_LINUX > - V9fsConf fsconf; > -#endif > - /* Max. number of ports we can have for a the virtio-serial device */ > - uint32_t max_virtserial_ports; > - virtio_net_conf net; > -} VirtIOPCIProxy; > - > /* virtio device */ > > static void virtio_pci_notify(void *opaque, uint16_t vector) > @@ -518,7 +499,7 @@ static const VirtIOBindings virtio_pci_bindings = { > .set_guest_notifiers = virtio_pci_set_guest_notifiers, > }; > > -static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, > +void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, > uint16_t vendor, uint16_t device, > uint16_t class_code, uint8_t pif) > { > @@ -677,23 +658,6 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) > return 0; > } > > -#ifdef CONFIG_VIRTFS > -static int virtio_9p_init_pci(PCIDevice *pci_dev) > -{ > - VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > - VirtIODevice *vdev; > - > - vdev = virtio_9p_init(&pci_dev->qdev,&proxy->fsconf); > - virtio_init_pci(proxy, vdev, > - PCI_VENDOR_ID_REDHAT_QUMRANET, > - 0x1009, > - 0x2, > - 0x00); > - > - return 0; > -} > -#endif > - > static PCIDeviceInfo virtio_info[] = { > { > .qdev.name = "virtio-blk-pci", > @@ -752,20 +716,7 @@ static PCIDeviceInfo virtio_info[] = { > DEFINE_PROP_END_OF_LIST(), > }, > .qdev.reset = virtio_pci_reset, > - },{ > -#ifdef CONFIG_VIRTFS > - .qdev.name = "virtio-9p-pci", > - .qdev.size = sizeof(VirtIOPCIProxy), > - .init = virtio_9p_init_pci, > - .qdev.props = (Property[]) { > - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > - DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag), > - DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id), > - DEFINE_PROP_END_OF_LIST(), > - }, > - }, { > -#endif > - /* end of list */ > + }, { /* end of list */ > } > }; > > diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h > new file mode 100644 > index 0000000..d9bf683 > --- /dev/null > +++ b/hw/virtio-pci.h > @@ -0,0 +1,42 @@ > +/* > + * Virtio PCI Bindings > + * > + * Copyright IBM, Corp. 2007 > + * Copyright (c) 2009 CodeSourcery > + * > + * Authors: > + * Anthony Liguori<aliguori@us.ibm.com> > + * Paul Brook<paul@codesourcery.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + */ > + > +#ifndef QEMU_VIRTIO_PCI_H > +#define QEMU_VIRTIO_PCI_H > + > +#include "virtio-net.h" > + > +typedef struct { > + PCIDevice pci_dev; > + VirtIODevice *vdev; > + uint32_t bugs; > + uint32_t addr; > + uint32_t class_code; > + uint32_t nvectors; > + BlockConf block; > + NICConf nic; > + uint32_t host_features; > +#ifdef CONFIG_LINUX > + V9fsConf fsconf; > +#endif > + /* Max. number of ports we can have for a the virtio-serial device */ > + uint32_t max_virtserial_ports; > + virtio_net_conf net; > +} VirtIOPCIProxy; > + > +extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, > + uint16_t vendor, uint16_t device, > + uint16_t class_code, uint8_t pif); > + > +#endif >
On Wed, 27 Apr 2011 08:57:48 -0700, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com> wrote: > On 04/27/2011 03:33 AM, Aneesh Kumar K.V wrote: > > On Wed, 27 Apr 2011 09:03:56 +0200, Jan Kiszka<jan.kiszka@web.de> wrote: > >> On 2011-04-27 08:53, Aneesh Kumar K.V wrote: > >>> Now that we start adding more files related to 9pfs > >>> it make sense to move them to a separate directory > >>> > >>> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com> > >>> --- > >>> Makefile.objs | 10 +++++++--- > >>> Makefile.target | 6 ++++-- > >>> configure | 2 ++ > >>> {hw => fsdev}/file-op-9p.h | 0 > >>> fsdev/qemu-fsdev.h | 2 +- > >>> hw/{ => 9pfs}/virtio-9p-debug.c | 0 > >>> hw/{ => 9pfs}/virtio-9p-debug.h | 0 > >>> hw/{ => 9pfs}/virtio-9p-local.c | 0 > >>> hw/{ => 9pfs}/virtio-9p-posix-acl.c | 2 +- > >>> hw/{ => 9pfs}/virtio-9p-xattr-user.c | 2 +- > >>> hw/{ => 9pfs}/virtio-9p-xattr.c | 2 +- > >>> hw/{ => 9pfs}/virtio-9p-xattr.h | 0 > >>> hw/{ => 9pfs}/virtio-9p.c | 0 > >>> hw/{ => 9pfs}/virtio-9p.h | 2 +- > >>> 14 files changed, 18 insertions(+), 10 deletions(-) > >>> rename {hw => fsdev}/file-op-9p.h (100%) > >>> rename hw/{ => 9pfs}/virtio-9p-debug.c (100%) > >>> rename hw/{ => 9pfs}/virtio-9p-debug.h (100%) > >>> rename hw/{ => 9pfs}/virtio-9p-local.c (100%) > >>> rename hw/{ => 9pfs}/virtio-9p-posix-acl.c (99%) > >>> rename hw/{ => 9pfs}/virtio-9p-xattr-user.c (98%) > >>> rename hw/{ => 9pfs}/virtio-9p-xattr.c (99%) > >>> rename hw/{ => 9pfs}/virtio-9p-xattr.h (100%) > >>> rename hw/{ => 9pfs}/virtio-9p.c (100%) > >> That's a good chance to split up this file, move virtio_9p_get_config > >> into a separate one and build the large virtio-9p.c as part of hwlib > >> while keeping the new file target-specific. I've some hack for this > >> lying around, but now that you are already at it... > >> > > How about doing the below patch also and move all those device specific > > stuff to virtio-9p-device.c and rest in virtio-9p.c ? > I think this goes on top of your old patch. I am ready to give pull to > Anthony with old patch. > May be this can go in next pull? > Yes the two new patches can go later -aneesh
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index daade77..11e87a3 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -14,6 +14,7 @@ #include "virtio.h" #include "pc.h" #include "qemu_socket.h" +#include "virtio-pci.h" #include "virtio-9p.h" #include "fsdev/qemu-fsdev.h" #include "virtio-9p-debug.h" @@ -3741,3 +3742,37 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf) return &s->vdev; } + +static int virtio_9p_init_pci(PCIDevice *pci_dev) +{ + VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); + VirtIODevice *vdev; + + vdev = virtio_9p_init(&pci_dev->qdev, &proxy->fsconf); + virtio_init_pci(proxy, vdev, + PCI_VENDOR_ID_REDHAT_QUMRANET, + 0x1009, + 0x2, + 0x00); + + return 0; +} + +static PCIDeviceInfo virtio_9p_info = { + .qdev.name = "virtio-9p-pci", + .qdev.size = sizeof(VirtIOPCIProxy), + .init = virtio_9p_init_pci, + .qdev.props = (Property[]) { + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), + DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag), + DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id), + DEFINE_PROP_END_OF_LIST(), + } +}; + +static void virtio_9p_register_devices(void) +{ + pci_qdev_register(&virtio_9p_info); +} + +device_init(virtio_9p_register_devices) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 729917d..f1377b1 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -25,6 +25,7 @@ #include "loader.h" #include "kvm.h" #include "blockdev.h" +#include "virtio-pci.h" /* from Linux's linux/virtio_pci.h */ @@ -90,26 +91,6 @@ */ #define wmb() do { } while (0) -/* PCI bindings. */ - -typedef struct { - PCIDevice pci_dev; - VirtIODevice *vdev; - uint32_t bugs; - uint32_t addr; - uint32_t class_code; - uint32_t nvectors; - BlockConf block; - NICConf nic; - uint32_t host_features; -#ifdef CONFIG_LINUX - V9fsConf fsconf; -#endif - /* Max. number of ports we can have for a the virtio-serial device */ - uint32_t max_virtserial_ports; - virtio_net_conf net; -} VirtIOPCIProxy; - /* virtio device */ static void virtio_pci_notify(void *opaque, uint16_t vector) @@ -518,7 +499,7 @@ static const VirtIOBindings virtio_pci_bindings = { .set_guest_notifiers = virtio_pci_set_guest_notifiers, }; -static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, +void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, uint16_t vendor, uint16_t device, uint16_t class_code, uint8_t pif) { @@ -677,23 +658,6 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) return 0; } -#ifdef CONFIG_VIRTFS -static int virtio_9p_init_pci(PCIDevice *pci_dev) -{ - VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); - VirtIODevice *vdev; - - vdev = virtio_9p_init(&pci_dev->qdev, &proxy->fsconf); - virtio_init_pci(proxy, vdev, - PCI_VENDOR_ID_REDHAT_QUMRANET, - 0x1009, - 0x2, - 0x00); - - return 0; -} -#endif - static PCIDeviceInfo virtio_info[] = { { .qdev.name = "virtio-blk-pci", @@ -752,20 +716,7 @@ static PCIDeviceInfo virtio_info[] = { DEFINE_PROP_END_OF_LIST(), }, .qdev.reset = virtio_pci_reset, - },{ -#ifdef CONFIG_VIRTFS - .qdev.name = "virtio-9p-pci", - .qdev.size = sizeof(VirtIOPCIProxy), - .init = virtio_9p_init_pci, - .qdev.props = (Property[]) { - DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), - DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag), - DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id), - DEFINE_PROP_END_OF_LIST(), - }, - }, { -#endif - /* end of list */ + }, { /* end of list */ } }; diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h new file mode 100644 index 0000000..d9bf683 --- /dev/null +++ b/hw/virtio-pci.h @@ -0,0 +1,42 @@ +/* + * Virtio PCI Bindings + * + * Copyright IBM, Corp. 2007 + * Copyright (c) 2009 CodeSourcery + * + * Authors: + * Anthony Liguori <aliguori@us.ibm.com> + * Paul Brook <paul@codesourcery.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + */ + +#ifndef QEMU_VIRTIO_PCI_H +#define QEMU_VIRTIO_PCI_H + +#include "virtio-net.h" + +typedef struct { + PCIDevice pci_dev; + VirtIODevice *vdev; + uint32_t bugs; + uint32_t addr; + uint32_t class_code; + uint32_t nvectors; + BlockConf block; + NICConf nic; + uint32_t host_features; +#ifdef CONFIG_LINUX + V9fsConf fsconf; +#endif + /* Max. number of ports we can have for a the virtio-serial device */ + uint32_t max_virtserial_ports; + virtio_net_conf net; +} VirtIOPCIProxy; + +extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, + uint16_t vendor, uint16_t device, + uint16_t class_code, uint8_t pif); + +#endif