[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 17/18] vfio-user: register handlers to facilitate migratio
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v5 17/18] vfio-user: register handlers to facilitate migration |
Date: |
Tue, 25 Jan 2022 15:48:16 +0000 |
On Wed, Jan 19, 2022 at 04:42:06PM -0500, Jagannathan Raman wrote:
> + * The client subsequetly asks the remote server for any data that
subsequently
> +static void vfu_mig_state_running(vfu_ctx_t *vfu_ctx)
> +{
> + VfuObject *o = vfu_get_private(vfu_ctx);
> + VfuObjectClass *k = VFU_OBJECT_GET_CLASS(OBJECT(o));
> + static int migrated_devs;
> + Error *local_err = NULL;
> + int ret;
> +
> + /**
> + * TODO: move to VFU_MIGR_STATE_RESUME handler. Presently, the
> + * VMSD data from source is not available at RESUME state.
> + * Working on a fix for this.
> + */
> + if (!o->vfu_mig_file) {
> + o->vfu_mig_file = qemu_fopen_ops(o, &vfu_mig_fops_load, false);
> + }
> +
> + ret = qemu_remote_loadvm(o->vfu_mig_file);
> + if (ret) {
> + VFU_OBJECT_ERROR(o, "vfu: failed to restore device state");
> + return;
> + }
> +
> + qemu_file_shutdown(o->vfu_mig_file);
> + o->vfu_mig_file = NULL;
> +
> + /* VFU_MIGR_STATE_RUNNING begins here */
> + if (++migrated_devs == k->nr_devs) {
When is this counter reset so migration can be tried again if it
fails/cancels?
> +static ssize_t vfu_mig_read_data(vfu_ctx_t *vfu_ctx, void *buf,
> + uint64_t size, uint64_t offset)
> +{
> + VfuObject *o = vfu_get_private(vfu_ctx);
> +
> + if (offset > o->vfu_mig_buf_size) {
> + return -1;
> + }
> +
> + if ((offset + size) > o->vfu_mig_buf_size) {
> + warn_report("vfu: buffer overflow - check pending_bytes");
> + size = o->vfu_mig_buf_size - offset;
> + }
> +
> + memcpy(buf, (o->vfu_mig_buf + offset), size);
> +
> + o->vfu_mig_buf_pending -= size;
This assumes that the caller increments offset by size each time. If
that assumption is okay, then we can just trust offset and don't need to
do arithmetic on vfu_mig_buf_pending. If that assumption is not correct,
then the code needs to be extended to safely update vfu_mig_buf_pending
when offset jumps around arbitrarily between calls.
> +uint64_t vmstate_vmsd_size(PCIDevice *pci_dev)
> +{
> + DeviceClass *dc = DEVICE_GET_CLASS(DEVICE(pci_dev));
> + const VMStateField *field = NULL;
> + uint64_t size = 0;
> +
> + if (!dc->vmsd) {
> + return 0;
> + }
> +
> + field = dc->vmsd->fields;
> + while (field && field->name) {
> + size += vmstate_size(pci_dev, field);
> + field++;
> + }
> +
> + return size;
> +}
This function looks incorrect because it ignores subsections as well as
runtime behavior during save(). Although VMStateDescription is partially
declarative, there is still a bunch of imperative code that can write to
the QEMUFile at save() time so there's no way of knowing the size ahead
of time.
I asked this in a previous revision of this series but I'm not sure if
it was answered: is it really necessary to know the size of the vmstate?
I thought the VFIO migration interface is designed to support
streaming reads/writes. We could choose a fixed size like 64KB and
stream the vmstate in 64KB chunks.
signature.asc
Description: PGP signature
- [PATCH v5 13/18] vfio-user: handle PCI config space accesses, (continued)
- [PATCH v5 13/18] vfio-user: handle PCI config space accesses, Jagannathan Raman, 2022/01/19
- [PATCH v5 12/18] vfio-user: run vfio-user context, Jagannathan Raman, 2022/01/19
- [PATCH v5 15/18] vfio-user: handle PCI BAR accesses, Jagannathan Raman, 2022/01/19
- [PATCH v5 14/18] vfio-user: handle DMA mappings, Jagannathan Raman, 2022/01/19
- [PATCH v5 16/18] vfio-user: handle device interrupts, Jagannathan Raman, 2022/01/19
- [PATCH v5 17/18] vfio-user: register handlers to facilitate migration, Jagannathan Raman, 2022/01/19
- Re: [PATCH v5 17/18] vfio-user: register handlers to facilitate migration,
Stefan Hajnoczi <=
[PATCH v5 18/18] vfio-user: avocado tests for vfio-user, Jagannathan Raman, 2022/01/19
Re: [PATCH v5 00/18] vfio-user server in QEMU, Stefan Hajnoczi, 2022/01/25