On Fri, May 16, 2025 at 10:35:44AM +0200, Cédric Le Goater wrote:
> On 5/12/25 17:32, Steve Sistare wrote:
> > cpr-transfer breaks vfio network connectivity to and from the guest, and
> > the host system log shows:
> > irq bypass consumer (token 00000000a03c32e5) registration fails: -16
> > which is EBUSY. This occurs because KVM descriptors are still open in
> > the old QEMU process. Close them.
> >
[1]
> > Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
>
> This patch doesn't build.
>
> /usr/bin/ld: libcommon.a.p/migration_cpr.c.o: in function `cpr_kvm_close':
> ./build/../migration/cpr.c:260: undefined reference to `kvm_close'
And it'll be also good if this patch can keep copying the kvm maintainer
(Paolo).. I have Paolo copied. So this patch is more of a kvm change not
migration, afaict. Maybe we should split this into two patches.
Steve, you could attach a cc line in this patch to make sure it won't be
forgotten when you repost (at [1] above, I think git-send-email would
remember that then):
Cc: Paolo Bonzini <pbonz...@redhat.com>
Some other questions below.
>
>
>
> Thanks,
>
> C.
>
>
>
> > ---
> > accel/kvm/kvm-all.c | 28 ++++++++++++++++++++++++++++
> > hw/vfio/helpers.c | 10 ++++++++++
> > include/hw/vfio/vfio-device.h | 2 ++
> > include/migration/cpr.h | 2 ++
> > include/qemu/vfio-helpers.h | 1 -
> > include/system/kvm.h | 1 +
> > migration/cpr-transfer.c | 18 ++++++++++++++++++
> > migration/cpr.c | 8 ++++++++
> > migration/migration.c | 1 +
> > 9 files changed, 70 insertions(+), 1 deletion(-)
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index 278a506..d619448 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -512,16 +512,23 @@ static int do_kvm_destroy_vcpu(CPUState *cpu)
> > goto err;
> > }
> > + /* If I am the CPU that created coalesced_mmio_ring, then discard it */
Are these "reset to NULL" below required or cleanup? It's not yet clear to
me when coalesced_mmio_ring isn't owned by the current CPU state. Maybe
also better to split this chunk with some commit message?
> > + if (s->coalesced_mmio_ring == (void *)cpu->kvm_run + PAGE_SIZE) {
> > + s->coalesced_mmio_ring = NULL;
> > + }
> > +
> > ret = munmap(cpu->kvm_run, mmap_size);
> > if (ret < 0) {
> > goto err;
> > }
> > + cpu->kvm_run = NULL;
> > if (cpu->kvm_dirty_gfns) {
> > ret = munmap(cpu->kvm_dirty_gfns, s->kvm_dirty_ring_bytes);
> > if (ret < 0) {
> > goto err;
> > }
> > + cpu->kvm_dirty_gfns = NULL;
> > }
> > kvm_park_vcpu(cpu);
> > @@ -600,6 +607,27 @@ err:
> > return ret;
> > }
> > +void kvm_close(void)
> > +{
> > + CPUState *cpu;
> > +
> > + CPU_FOREACH(cpu) {
> > + cpu_remove_sync(cpu);
> > + close(cpu->kvm_fd);
> > + cpu->kvm_fd = -1;
> > + close(cpu->kvm_vcpu_stats_fd);
> > + cpu->kvm_vcpu_stats_fd = -1;
> > + }
> > +
> > + if (kvm_state && kvm_state->fd != -1) {
> > + close(kvm_state->vmfd);
> > + kvm_state->vmfd = -1;
> > + close(kvm_state->fd);
> > + kvm_state->fd = -1;
> > + }
> > + kvm_state = NULL;
> > +}
> > +
> > /*
> > * dirty pages logging control
> > */
> > diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
> > index d0dbab1..af1db2f 100644
> > --- a/hw/vfio/helpers.c
> > +++ b/hw/vfio/helpers.c
> > @@ -117,6 +117,16 @@ bool vfio_get_info_dma_avail(struct
> > vfio_iommu_type1_info *info,
> > int vfio_kvm_device_fd = -1;
> > #endif
> > +void vfio_kvm_device_close(void)
> > +{
> > +#ifdef CONFIG_KVM
> > + if (vfio_kvm_device_fd != -1) {
> > + close(vfio_kvm_device_fd);
> > + vfio_kvm_device_fd = -1;
> > + }
> > +#endif
> > +}
> > +
> > int vfio_kvm_device_add_fd(int fd, Error **errp)
> > {
> > #ifdef CONFIG_KVM
> > diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> > index 4e4d0b6..6eb6f21 100644
> > --- a/include/hw/vfio/vfio-device.h
> > +++ b/include/hw/vfio/vfio-device.h
> > @@ -231,4 +231,6 @@ void vfio_device_set_fd(VFIODevice *vbasedev, const
> > char *str, Error **errp);
> > void vfio_device_init(VFIODevice *vbasedev, int type, VFIODeviceOps *ops,
> > DeviceState *dev, bool ram_discard);
> > int vfio_device_get_aw_bits(VFIODevice *vdev);
> > +
> > +void vfio_kvm_device_close(void);
> > #endif /* HW_VFIO_VFIO_COMMON_H */
> > diff --git a/include/migration/cpr.h b/include/migration/cpr.h
> > index fc6aa33..5f1ff10 100644
> > --- a/include/migration/cpr.h
> > +++ b/include/migration/cpr.h
> > @@ -31,7 +31,9 @@ void cpr_state_close(void);
> > struct QIOChannel *cpr_state_ioc(void);
> > bool cpr_needed_for_reuse(void *opaque);
> > +void cpr_kvm_close(void);
> > +void cpr_transfer_init(void);
> > QEMUFile *cpr_transfer_output(MigrationChannel *channel, Error **errp);
> > QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp);
> > diff --git a/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h
> > index bde9495..a029036 100644
> > --- a/include/qemu/vfio-helpers.h
> > +++ b/include/qemu/vfio-helpers.h
> > @@ -28,5 +28,4 @@ void qemu_vfio_pci_unmap_bar(QEMUVFIOState *s, int index,
> > void *bar,
> > uint64_t offset, uint64_t size);
> > int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
> > int irq_type, Error **errp);
> > -
> > #endif
> > diff --git a/include/system/kvm.h b/include/system/kvm.h
> > index b690dda..cfaa94c 100644
> > --- a/include/system/kvm.h
> > +++ b/include/system/kvm.h
> > @@ -194,6 +194,7 @@ bool kvm_has_sync_mmu(void);
> > int kvm_has_vcpu_events(void);
> > int kvm_max_nested_state_length(void);
> > int kvm_has_gsi_routing(void);
> > +void kvm_close(void);
> > /**
> > * kvm_arm_supports_user_irq
> > diff --git a/migration/cpr-transfer.c b/migration/cpr-transfer.c
> > index e1f1403..396558f 100644
> > --- a/migration/cpr-transfer.c
> > +++ b/migration/cpr-transfer.c
> > @@ -17,6 +17,24 @@
> > #include "migration/vmstate.h"
> > #include "trace.h"
> > +static int cpr_transfer_notifier(NotifierWithReturn *notifier,
> > + MigrationEvent *e,
> > + Error **errp)
> > +{
> > + if (e->type == MIG_EVENT_PRECOPY_DONE) {
> > + cpr_kvm_close();
> > + }
> > + return 0;
> > +}
> > +
> > +void cpr_transfer_init(void)
> > +{
> > + static NotifierWithReturn notifier;
> > +
> > + migration_add_notifier_mode(¬ifier, cpr_transfer_notifier,
> > + MIG_MODE_CPR_TRANSFER);
> > +}
> > +
> > QEMUFile *cpr_transfer_output(MigrationChannel *channel, Error **errp)
> > {
> > MigrationAddress *addr = channel->addr;
> > diff --git a/migration/cpr.c b/migration/cpr.c
> > index 0b01e25..6102d04 100644
> > --- a/migration/cpr.c
> > +++ b/migration/cpr.c
> > @@ -7,12 +7,14 @@
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > +#include "hw/vfio/vfio-device.h"
> > #include "migration/cpr.h"
> > #include "migration/misc.h"
> > #include "migration/options.h"
> > #include "migration/qemu-file.h"
> > #include "migration/savevm.h"
> > #include "migration/vmstate.h"
> > +#include "system/kvm.h"
> > #include "system/runstate.h"
> > #include "trace.h"
> > @@ -252,3 +254,9 @@ bool cpr_needed_for_reuse(void *opaque)
> > MigMode mode = migrate_mode();
> > return mode == MIG_MODE_CPR_TRANSFER;
> > }
> > +
> > +void cpr_kvm_close(void)
> > +{
> > + kvm_close();
> > + vfio_kvm_device_close();
> > +}
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 4697732..89e2026 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -337,6 +337,7 @@ void migration_object_init(void)
> > ram_mig_init();
> > dirty_bitmap_mig_init();
> > + cpr_transfer_init();
> > /* Initialize cpu throttle timers */
> > cpu_throttle_init();
>
--
Peter Xu
