[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v4 PATCH 41/49] multi-process/mig: Enable VMSD save in the Prox
From: |
Daniel P . Berrangé |
Subject: |
Re: [RFC v4 PATCH 41/49] multi-process/mig: Enable VMSD save in the Proxy object |
Date: |
Wed, 13 Nov 2019 15:50:19 +0000 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Thu, Oct 24, 2019 at 05:09:22AM -0400, Jagannathan Raman wrote:
> Collect the VMSD from remote process on the source and save
> it to the channel leading to the destination
>
> Signed-off-by: Elena Ufimtseva <address@hidden>
> Signed-off-by: John G Johnson <address@hidden>
> Signed-off-by: Jagannathan Raman <address@hidden>
> ---
> New patch in v4
>
> hw/proxy/qemu-proxy.c | 132
> ++++++++++++++++++++++++++++++++++++++++++
> include/hw/proxy/qemu-proxy.h | 2 +
> include/io/mpqemu-link.h | 1 +
> 3 files changed, 135 insertions(+)
>
> diff --git a/hw/proxy/qemu-proxy.c b/hw/proxy/qemu-proxy.c
> index 623a6c5..ce72e6a 100644
> --- a/hw/proxy/qemu-proxy.c
> +++ b/hw/proxy/qemu-proxy.c
> @@ -52,6 +52,14 @@
> #include "util/event_notifier-posix.c"
> #include "hw/boards.h"
> #include "include/qemu/log.h"
> +#include "io/channel.h"
> +#include "migration/qemu-file-types.h"
> +#include "qapi/error.h"
> +#include "io/channel-util.h"
> +#include "migration/qemu-file-channel.h"
> +#include "migration/qemu-file.h"
> +#include "migration/migration.h"
> +#include "migration/vmstate.h"
>
> QEMUTimer *hb_timer;
> static void pci_proxy_dev_realize(PCIDevice *dev, Error **errp);
> @@ -62,6 +70,9 @@ static void stop_heartbeat_timer(void);
> static void childsig_handler(int sig, siginfo_t *siginfo, void *ctx);
> static void broadcast_msg(MPQemuMsg *msg, bool need_reply);
>
> +#define PAGE_SIZE getpagesize()
> +uint8_t *mig_data;
> +
> static void childsig_handler(int sig, siginfo_t *siginfo, void *ctx)
> {
> /* TODO: Add proper handler. */
> @@ -357,14 +368,135 @@ static void pci_proxy_dev_inst_init(Object *obj)
> dev->mem_init = false;
> }
>
> +typedef struct {
> + QEMUFile *rem;
> + PCIProxyDev *dev;
> +} proxy_mig_data;
> +
> +static void *proxy_mig_out(void *opaque)
> +{
> + proxy_mig_data *data = opaque;
> + PCIProxyDev *dev = data->dev;
> + uint8_t byte;
> + uint64_t data_size = PAGE_SIZE;
> +
> + mig_data = g_malloc(data_size);
> +
> + while (true) {
> + byte = qemu_get_byte(data->rem);
There is a pretty large set of APIs hiding behind the qemu_get_byte
call, which does not give me confidence that...
> + mig_data[dev->migsize++] = byte;
> + if (dev->migsize == data_size) {
> + data_size += PAGE_SIZE;
> + mig_data = g_realloc(mig_data, data_size);
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static int proxy_pre_save(void *opaque)
> +{
> + PCIProxyDev *pdev = opaque;
> + proxy_mig_data *mig_data;
> + QEMUFile *f_remote;
> + MPQemuMsg msg = {0};
> + QemuThread thread;
> + Error *err = NULL;
> + QIOChannel *ioc;
> + uint64_t size;
> + int fd[2];
> +
> + if (socketpair(AF_UNIX, SOCK_STREAM, 0, fd)) {
> + return -1;
> + }
> +
> + ioc = qio_channel_new_fd(fd[0], &err);
> + if (err) {
> + error_report_err(err);
> + return -1;
> + }
> +
> + qio_channel_set_name(QIO_CHANNEL(ioc), "PCIProxyDevice-mig");
> +
> + f_remote = qemu_fopen_channel_input(ioc);
> +
> + pdev->migsize = 0;
> +
> + mig_data = g_malloc0(sizeof(proxy_mig_data));
> + mig_data->rem = f_remote;
> + mig_data->dev = pdev;
> +
> + qemu_thread_create(&thread, "Proxy MIG_OUT", proxy_mig_out, mig_data,
> + QEMU_THREAD_DETACHED);
> +
> + msg.cmd = START_MIG_OUT;
> + msg.bytestream = 0;
> + msg.num_fds = 2;
> + msg.fds[0] = fd[1];
> + msg.fds[1] = GET_REMOTE_WAIT;
> +
> + mpqemu_msg_send(pdev->mpqemu_link, &msg, pdev->mpqemu_link->com);
> + size = wait_for_remote(msg.fds[1]);
> + PUT_REMOTE_WAIT(msg.fds[1]);
> +
> + assert(size != ULLONG_MAX);
> +
> + /*
> + * migsize is being update by a separate thread. Using volatile to
> + * instruct the compiler to fetch the value of this variable from
> + * memory during every read
> + */
> + while (*((volatile uint64_t *)&pdev->migsize) < size) {
> + }
> +
> + qemu_thread_cancel(&thread);
....this is a safe way to stop the thread executing without
resulting in memory being leaked.
In addition thread cancellation is asynchronous, so the thread
may still be using the QEMUFile object while....
> + qemu_fclose(f_remote);
..this is closing it. This feels like it is a crash danger.
> + close(fd[1]);
> +
> + return 0;
> +}
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- Re: [RFC v4 PATCH 41/49] multi-process/mig: Enable VMSD save in the Proxy object,
Daniel P . Berrangé <=