qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 04/10] vdpa-dev: implement the instance_init/class_init in


From: Stefano Garzarella
Subject: Re: [PATCH v2 04/10] vdpa-dev: implement the instance_init/class_init interface
Date: Mon, 7 Mar 2022 10:13:21 +0100

On Mon, Mar 07, 2022 at 08:55:35AM +0000, Longpeng (Mike, Cloud Infrastructure 
Service Product Dept.) wrote:


-----Original Message-----
From: Stefano Garzarella [mailto:sgarzare@redhat.com]
Sent: Monday, March 7, 2022 4:10 PM
To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
<longpeng2@huawei.com>
Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com;
pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan
<yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>;
qemu-devel@nongnu.org
Subject: Re: [PATCH v2 04/10] vdpa-dev: implement the instance_init/class_init
interface

Hi Longpeng,

On Sat, Mar 05, 2022 at 06:06:42AM +0000, Longpeng (Mike, Cloud Infrastructure
Service Product Dept.) wrote:
>Hi Stefano,
>
>> -----Original Message-----
>> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
>> Sent: Wednesday, January 19, 2022 7:24 PM
>> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
>> <longpeng2@huawei.com>
>> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com;
>> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan
>> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>;
>> qemu-devel@nongnu.org
>> Subject: Re: [PATCH v2 04/10] vdpa-dev: implement the
instance_init/class_init
>> interface
>>
>> On Mon, Jan 17, 2022 at 08:43:25PM +0800, Longpeng(Mike) via wrote:
>> >From: Longpeng <longpeng2@huawei.com>
>> >
>> >Implements the .instance_init and the .class_init interface.
>> >
>> >Signed-off-by: Longpeng <longpeng2@huawei.com>
>> >---
>> > hw/virtio/vdpa-dev-pci.c     | 52 ++++++++++++++++++++++-
>> > hw/virtio/vdpa-dev.c         | 81 +++++++++++++++++++++++++++++++++++-
>> > include/hw/virtio/vdpa-dev.h |  5 +++
>> > 3 files changed, 134 insertions(+), 4 deletions(-)
>> >
>> >diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c
>> >index a5a7b528a9..257538dbdd 100644
>> >--- a/hw/virtio/vdpa-dev-pci.c
>> >+++ b/hw/virtio/vdpa-dev-pci.c
>> >@@ -25,12 +25,60 @@ struct VhostVdpaDevicePCI {
>> >
>> > static void vhost_vdpa_device_pci_instance_init(Object *obj)
>> > {
>> >-    return;
>> >+    VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(obj);
>> >+
>> >+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>> >+                                TYPE_VHOST_VDPA_DEVICE);
>> >+    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
>> >+                              "bootindex");
>> >+}
>> >+
>> >+static Property vhost_vdpa_device_pci_properties[] = {
>> >+    DEFINE_PROP_END_OF_LIST(),
>> >+};
>> >+
>> >+static void
>> >+vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>> >+{
>> >+    VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev);
>> >+    DeviceState *vdev = DEVICE(&dev->vdev);
>> >+    uint32_t vdev_id;
>> >+    uint32_t num_queues;
>> >+    int fd;
>> >+
>> >+    fd = qemu_open(dev->vdev.vdpa_dev, O_RDWR, errp);
>>
>> We should use `vdpa_dev_fd` if the user set it, and I think we should
>> also check that `vdpa_dev` is not null.
>>
>
>The user can set both 'vdpa_dev_fd' and 'vdpa_dev' now, but how
>to make sure the 'vdpa_dev_fd' is really a FD of the 'vdpa_dev' ?
>Maybe we should remove 'vdpa_dev_fd' from
>'vhost_vdpa_device_properties',
>so the user can only set 'vdpa_dev'.

This is the same problem that would happen if the user passed a path any
file or device (e.g. /dev/null). I believe that on the first operation
on it (e.g. an ioctl) we would get an error and exit.


Yes, but how about the 'vdpa_dev_fd' refers to /dev/vhost-vdpa-0 but
the 'vdpa_dev' refers to /dev/vhost-vdpa-1 ? Should we need to consider
this case ?

I think we can do as we already do in vhost-scsi and vhost-vsock. If fd is set we use that otherwise we use path.

If we want we can also print an error and exit if both are set, since IMHO should be set only one of the two.

Thanks,
Stefano




reply via email to

[Prev in Thread] Current Thread [Next in Thread]