> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> Sent: Wednesday, January 5, 2022 6:01 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpe...@huawei.com>
> Cc: m...@redhat.com; jasow...@redhat.com; sgarz...@redhat.com;
> coh...@redhat.com; pbonz...@redhat.com; Gonglei (Arei)
> <arei.gong...@huawei.com>; Yechuan <yech...@huawei.com>; Huangzhichao
> <huangzhic...@huawei.com>; qemu-devel@nongnu.org
> Subject: Re: [RFC 04/10] vdpa-dev: implement the instance_init/class_init
> interface
>
> On Wed, Jan 05, 2022 at 08:58:54AM +0800, Longpeng(Mike) wrote:
> > From: Longpeng <longpe...@huawei.com>
> >
> > Implements the .instance_init and the .class_init interface.
> >
> > Signed-off-by: Longpeng <longpe...@huawei.com>
> > ---
> > hw/virtio/vdpa-dev-pci.c | 80 +++++++++++++++++++++++++++++++++++-
> > hw/virtio/vdpa-dev.c | 68 +++++++++++++++++++++++++++++-
> > include/hw/virtio/vdpa-dev.h | 2 +
> > 3 files changed, 146 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c
> > index a5a7b528a9..0af54a26d4 100644
> > --- a/hw/virtio/vdpa-dev-pci.c
> > +++ b/hw/virtio/vdpa-dev-pci.c
> > @@ -23,14 +23,90 @@ struct VhostVdpaDevicePCI {
> > VhostVdpaDevice vdev;
> > };
> >
> > +static uint32_t
> > +vdpa_dev_pci_get_info(const char *name, uint64_t cmd, Error **errp)
>
> vdpa_dev_pci_get_u32() might be a clearer name.
>
OK.
> > +{
> > + int device_fd;
> > + uint32_t val;
> > + int ret;
> > +
> > + device_fd = qemu_open(name, O_RDWR, errp);
> > + if (device_fd == -1) {
> > + return (uint32_t)-1;
> > + }
> > +
> > + ret = ioctl(device_fd, cmd, &val);
> > + if (ret < 0) {
> > + error_setg(errp, "vhost-vdpa-device-pci: cmd 0x%lx failed: %s",
> > + cmd, strerror(errno));
> > + goto out;
> > + }
> > +
> > +out:
> > + close(device_fd);
>
> Race conditions are possible if the device node is replaced between
> calls. Why not open the file once and reuse the fd across ioctl calls?
>
> Both VhostVdpaDevicePCI and VhostVdpaDevice need the fd but
> VhostVdpaDevicePCI needs it first. I suggest passing ownership of the fd
> to the VhostVdpaDevice. One way of doing this is using QOM properties so
> that VhostVdpaDevice can use the given fd instead of reopening the file.
> (If fd is -1 then VhostVdpaDevice can try to open the file. That is
> necessary when VhostVdpaDevice is used directly with virtio-mmio because
> there is no proxy object.)
Adding the fd field into the VhostVdpaDevice looks fine! I'll do it in V2.
Thanks.
