On Mon, Jun 30, 2014 at 11:55:46AM +0200, Marc Marí wrote:
> +static void qvirtio_pci_foreach_callback(
> + QPCIDevice *dev, int devfn, void *data)
> +{
> + QVirtioPCIForeachData *d = data;
> + QVirtioPCIDevice *vpcidev = qpcidevice_to_qvirtiodevice(dev);
> +
> + if (vpcidev->vdev.device_type == d->device_type) {
> + d->func(&vpcidev->vdev, d->user_data);
> + }
We should think about memory management here. Who will free vpcidev?
In the case where device_type matches, ->func() should be responsible
for the virtio device it receives.
In the case where device_type does not match, we must free vpcidev.
> +}
> +
> +static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
> +{
> + QVirtioPCIDevice *vpcidev = data;
> + vpcidev->pdev = ((QVirtioPCIDevice *)d)->pdev;
> + vpcidev->vdev.device_type = ((QVirtioPCIDevice *)d)->vdev.device_type;
> +}
This function may need to g_free(d) once you've settled on a memory
management rules for virtio-pci.c.
> +typedef struct QVirtioPCIForeachData {
> + void (*func)(QVirtioDevice *d, void *data);
> + uint16_t device_type;
> + void *user_data;
> +} QVirtioPCIForeachData;
This is only used within virtio-pci.c. It should be declared there too
since it doesn't need to be public.
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> new file mode 100644
> index 0000000..52f6b5b
> --- /dev/null
> +++ b/tests/libqos/virtio.h
> @@ -0,0 +1,28 @@
> +/*
> + * libqos virtio definitions
> + *
> + * Copyright (c) 2014 Marc Marí
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_VIRTIO_H
> +#define LIBQOS_VIRTIO_H
> +
> +#define QVIRTQUEUE_MAX_SIZE 1024
Unused, please drop.
> +#define QVIRTIO_VENDOR_ID 0x1AF4
> +
> +#define QVIRTIO_NET_DEVICE_ID 0x1
> +#define QVIRTIO_BLK_DEVICE_ID 0x2
> +
> +typedef struct QVirtioDevice QVirtioDevice;
> +typedef struct QVirtQueue QVirtQueue;
> +typedef struct QVRing QVRing;
QVirtQueue and QVRing are unused. Please drop them.
> +static void pci_basic(void)
> +{
> + QVirtioPCIDevice *dev;
> + QPCIBus *bus;
> +
> + bus = test_start();
> +
> + dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID);
> + g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
> + g_assert_cmphex(dev->pdev->devfn, ==, ((PCI_SLOT << 3) | PCI_FN));
Should dev be freed once we are done with it?
pgppvCERYuSb_.pgp
Description: PGP signature
