>-----Original Message-----
>From: Steve Sistare <steven.sist...@oracle.com>
>Subject: [PATCH V3 36/42] vfio/iommufd: preserve descriptors
>
>Save the iommu and vfio device fd in CPR state when it is created.
>After CPR, the fd number is found in CPR state and reused. Remember
>the reused status for subsequent patches. The reused status is cleared
>when vmstate load finishes.
>
>Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
>---
> backends/iommufd.c | 19 ++++++++++---------
> hw/vfio/cpr-iommufd.c | 16 ++++++++++++++++
> hw/vfio/device.c | 10 ++--------
> hw/vfio/iommufd.c | 13 +++++++++++--
> include/system/iommufd.h | 1 +
> 5 files changed, 40 insertions(+), 19 deletions(-)
>
>diff --git a/backends/iommufd.c b/backends/iommufd.c
>index 6fed1c1..492747c 100644
>--- a/backends/iommufd.c
>+++ b/backends/iommufd.c
>@@ -16,12 +16,18 @@
> #include "qemu/module.h"
> #include "qom/object_interfaces.h"
> #include "qemu/error-report.h"
>+#include "migration/cpr.h"
> #include "monitor/monitor.h"
> #include "trace.h"
> #include "hw/vfio/vfio-device.h"
> #include <sys/ioctl.h>
> #include <linux/iommufd.h>
>
>+static const char *iommufd_fd_name(IOMMUFDBackend *be)
>+{
>+ return object_get_canonical_path_component(OBJECT(be));
>+}
>+
> static void iommufd_backend_init(Object *obj)
> {
> IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
>@@ -47,9 +53,8 @@ static void iommufd_backend_set_fd(Object *obj, const
>char *str, Error **errp)
> IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
> int fd = -1;
>
>- fd = monitor_fd_param(monitor_cur(), str, errp);
>+ fd = cpr_get_fd_param(iommufd_fd_name(be), str, 0, &be->cpr_reused, errp);
> if (fd == -1) {
>- error_prepend(errp, "Could not parse remote object fd %s:", str);
> return;
> }
> be->fd = fd;
>@@ -95,14 +100,9 @@ bool iommufd_change_process(IOMMUFDBackend *be,
>Error **errp)
>
> bool iommufd_backend_connect(IOMMUFDBackend *be, Error **errp)
> {
>- int fd;
>-
> if (be->owned && !be->users) {
>- fd = qemu_open("/dev/iommu", O_RDWR, errp);
>- if (fd < 0) {
>- return false;
>- }
>- be->fd = fd;
>+ be->fd = cpr_open_fd("/dev/iommu", O_RDWR, iommufd_fd_name(be), 0,
>+ &be->cpr_reused, errp);
Need to check error before assign to be->fd.
> }
> be->users++;
>
>@@ -121,6 +121,7 @@ void iommufd_backend_disconnect(IOMMUFDBackend
>*be)
> be->fd = -1;
> }
> out:
>+ cpr_delete_fd(iommufd_fd_name(be), 0);
> trace_iommufd_backend_disconnect(be->fd, be->users);
> }
>
>diff --git a/hw/vfio/cpr-iommufd.c b/hw/vfio/cpr-iommufd.c
>index 46f2006..b760bd3 100644
>--- a/hw/vfio/cpr-iommufd.c
>+++ b/hw/vfio/cpr-iommufd.c
>@@ -8,6 +8,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "hw/vfio/vfio-cpr.h"
>+#include "hw/vfio/vfio-device.h"
> #include "migration/blocker.h"
> #include "migration/cpr.h"
> #include "migration/migration.h"
>@@ -25,10 +26,25 @@ static bool vfio_cpr_supported(VFIOIOMMUFDContainer
>*container, Error **errp)
> return true;
> }
>
>+static int vfio_container_post_load(void *opaque, int version_id)
>+{
>+ VFIOIOMMUFDContainer *container = opaque;
>+ VFIOContainerBase *bcontainer = &container->bcontainer;
>+ VFIODevice *vbasedev;
>+
>+ QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>+ vbasedev->cpr.reused = false;
>+ }
>+ container->be->cpr_reused = false;
It's strange to set iommufd and vfio device's reused in container's post load,
Maybe better to do it in their own post load handler?
>+
>+ return 0;
>+}
>+
> static const VMStateDescription vfio_container_vmstate = {
> .name = "vfio-iommufd-container",
> .version_id = 0,
> .minimum_version_id = 0,
>+ .post_load = vfio_container_post_load,
> .needed = cpr_needed_for_reuse,
> .fields = (VMStateField[]) {
> VMSTATE_END_OF_LIST()
>diff --git a/hw/vfio/device.c b/hw/vfio/device.c
>index 8e9de68..02f384e 100644
>--- a/hw/vfio/device.c
>+++ b/hw/vfio/device.c
>@@ -312,14 +312,8 @@ bool vfio_device_get_name(VFIODevice *vbasedev,
>Error **errp)
>
> void vfio_device_set_fd(VFIODevice *vbasedev, const char *str, Error **errp)
> {
>- ERRP_GUARD();
>- int fd = monitor_fd_param(monitor_cur(), str, errp);
>-
>- if (fd < 0) {
>- error_prepend(errp, "Could not parse remote object fd %s:", str);
>- return;
>- }
>- vbasedev->fd = fd;
>+ vbasedev->fd = cpr_get_fd_param(vbasedev->dev->id, str, 0,
>+ &vbasedev->cpr.reused, errp);
Same here.
> }
>
> static VFIODeviceIOOps vfio_device_io_ops_ioctl;
>diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>index dabb948..046f601 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -26,6 +26,7 @@
> #include "qemu/cutils.h"
> #include "qemu/chardev_open.h"
> #include "migration/blocker.h"
>+#include "migration/cpr.h"
> #include "pci.h"
> #include "vfio-iommufd.h"
> #include "vfio-helpers.h"
>@@ -530,13 +531,18 @@ static bool iommufd_cdev_attach(const char *name,
>VFIODevice *vbasedev,
>
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
>
> if (vbasedev->fd < 0) {
>- devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
>+ devfd = cpr_find_fd(vbasedev->name, 0);
>+ vbasedev->cpr.reused = (devfd >= 0);
>+ if (!vbasedev->cpr.reused) {
>+ devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
>+ }
> if (devfd < 0) {
> return false;
> }
> vbasedev->fd = devfd;
> } else {
> devfd = vbasedev->fd;
>+ /* reused was set in iommufd_backend_set_fd */
Should be vfio_device_set_fd
> }
>
> if (!iommufd_cdev_connect_and_bind(vbasedev, errp)) {
>@@ -634,7 +640,9 @@ found_container:
>
> vfio_device_prepare(vbasedev, bcontainer, &dev_info);
> vfio_iommufd_cpr_register_device(vbasedev);
>-
>+ if (!vbasedev->cpr.reused) {
>+ cpr_save_fd(vbasedev->name, 0, vbasedev->fd);
>+ }
> trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev-
>>num_irqs,
> vbasedev->num_regions, vbasedev->flags);
> return true;
>@@ -673,6 +681,7 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
>
> migrate_del_blocker(&vbasedev->cpr.id_blocker);
> vfio_iommufd_cpr_unregister_device(vbasedev);
>+ cpr_delete_fd(vbasedev->name, 0);
> iommufd_cdev_unbind_and_disconnect(vbasedev);
> close(vbasedev->fd);
> }
>diff --git a/include/system/iommufd.h b/include/system/iommufd.h
>index db9ed53..5c17abd 100644
>--- a/include/system/iommufd.h
>+++ b/include/system/iommufd.h
>@@ -32,6 +32,7 @@ struct IOMMUFDBackend {
> /*< protected >*/
> int fd; /* /dev/iommu file descriptor */
> bool owned; /* is the /dev/iommu opened internally */
>+ bool cpr_reused; /* fd is reused after CPR */
> uint32_t users;
>
> /*< public >*/
>--
>1.8.3.1
