[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 42/50] multi-process/mig: Send VMSD of remote to the Proxy
From: |
Elena Ufimtseva |
Subject: |
Re: [PATCH v5 42/50] multi-process/mig: Send VMSD of remote to the Proxy object |
Date: |
Thu, 5 Mar 2020 08:32:28 -0800 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
On Thu, Mar 05, 2020 at 02:39:49PM +0000, Dr. David Alan Gilbert wrote:
> * Jagannathan Raman (address@hidden) wrote:
> > The remote process sends the VMSD to the Proxy object, on the source
> > side
> >
> > Signed-off-by: Elena Ufimtseva <address@hidden>
> > Signed-off-by: John G Johnson <address@hidden>
> > Signed-off-by: Jagannathan Raman <address@hidden>
> > ---
> > migration/savevm.c | 27 +++++++++++++++++++++++++++
> > migration/savevm.h | 2 ++
> > remote/remote-main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 72 insertions(+)
> >
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 1d4220e..09af14d 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2942,3 +2942,30 @@ bool vmstate_check_only_migratable(const
> > VMStateDescription *vmsd)
> >
> > return !(vmsd && vmsd->unmigratable);
> > }
> > +
>
> Can we add something here commenting, e.g.
> /* Called by the remote process to serialise migration back to the qemu
> * */
Will add this.
> > +int qemu_remote_savevm(QEMUFile *f)
> > +{
> > + SaveStateEntry *se;
> > + int ret;
> > +
> > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> > + if (!se->vmsd || !vmstate_save_needed(se->vmsd, se->opaque)) {
> > + continue;
> > + }
> > +
> > + save_section_header(f, se, QEMU_VM_SECTION_FULL);
> > +
> > + ret = vmstate_save(f, se, NULL);
> > + if (ret) {
> > + qemu_file_set_error(f, ret);
> > + return ret;
> > + }
> > +
> > + save_section_footer(f, se);
> > + }
> > +
> > + qemu_put_byte(f, QEMU_VM_EOF);
> > + qemu_fflush(f);
>
> You have a qemu_fflush in process_start_mig_out just after you call it
> - so you don't need both; I suggest you remove this one.
>
Ok.
> > + return 0;
>
> And make this return qemu_file_get_error(f); just like
> qemu_save_device_state and then makybe some day we can merge them.
>
Will do.
> > +}
>
>
> > diff --git a/migration/savevm.h b/migration/savevm.h
> > index ba64a7e..0491d3a 100644
> > --- a/migration/savevm.h
> > +++ b/migration/savevm.h
> > @@ -65,4 +65,6 @@ void qemu_loadvm_state_cleanup(void);
> > int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> > int qemu_load_device_state(QEMUFile *f);
> >
> > +int qemu_remote_savevm(QEMUFile *f);
> > +
> > #endif
> > diff --git a/remote/remote-main.c b/remote/remote-main.c
> > index 58d9905..e97eb76 100644
> > --- a/remote/remote-main.c
> > +++ b/remote/remote-main.c
> > @@ -53,6 +53,16 @@
> > #include "qemu/log.h"
> > #include "qemu/cutils.h"
> > #include "remote-opts.h"
> > +#include "qapi/error.h"
> > +#include "io/channel-util.h"
> > +
> > +#include "io/channel.h"
> > +#include "io/channel-socket.h"
> > +#include "migration/qemu-file-types.h"
> > +#include "migration/savevm.h"
> > +#include "migration/qemu-file-channel.h"
> > +#include "migration/qemu-file.h"
> > +
> > #include "monitor/monitor.h"
> > #include "chardev/char.h"
> > #include "sysemu/reset.h"
> > @@ -322,6 +332,36 @@ static int setup_device(MPQemuMsg *msg, Error **errp)
> >
> > }
> >
> > +static void process_start_mig_out(MPQemuMsg *msg)
> > +{
> > + int wait = msg->fds[1];
> > + Error *err = NULL;
> > + QIOChannel *ioc;
> > + QEMUFile *f;
> > +
> > + ioc = qio_channel_new_fd(msg->fds[0], &err);
> > + if (err) {
> > + error_report_err(err);
> > + return;
> > + }
> > +
> > + qio_channel_set_name(QIO_CHANNEL(ioc), "remote-migration-channel");
> > +
> > + f = qemu_fopen_channel_output(ioc);
> > +
> > + bdrv_drain_all();
> > + (void)bdrv_flush_all();
>
> Do remote process always have block code? I mean can't we have a remote
> process that's just say a NIC ?
Not always (in the future), we will account for this.
>
> > + (void)qemu_remote_savevm(f);
>
> It's probably bad to ignore errors here; what you could do is if there's
> an error, you shoul dprint something and then send a poison value back
> to the QEMU to let it know that you've failed.
>
Yes, will add this.
> > + qemu_fflush(f);
> > +
> > + notify_proxy(wait, (uint64_t)qemu_ftell(f));
> > + PUT_REMOTE_WAIT(wait);
> > +
> > + qemu_fclose(f);
> > +}
> > +
> > static void process_msg(GIOCondition cond, MPQemuChannel *chan)
> > {
> > MPQemuMsg *msg = NULL;
> > @@ -411,6 +451,9 @@ static void process_msg(GIOCondition cond,
> > MPQemuChannel *chan)
> > notify_proxy(msg->fds[0], 0);
> > }
> > break;
> > + case START_MIG_OUT:
> > + process_start_mig_out(msg);
> > + break;
> > default:
> > error_setg(&err, "Unknown command");
> > goto finalize_loop;
> > --
> > 1.8.3.1
>
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>