[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] migration: Test for disabled features on re
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] migration: Test for disabled features on reception |
Date: |
Tue, 15 Nov 2016 10:42:03 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
* Juan Quintela (address@hidden) wrote:
> "Dr. David Alan Gilbert" <address@hidden> wrote:
> > * Juan Quintela (address@hidden) wrote:
> >> Right now, if we receive a compressed page or a xbzrle page while this
> >> features are disabled, Bad Things (TM) can happen. Just add a test for
> >> them.
> >
> > This confuses me; I didn't think until recently we could
> > guarantee anything about having the capabilities set on the destination
> > side. Until -incoming defer the destination didn't have a way of setting
> > capabilities in a known state before starting the reception.
>
> Ouch.
>
> So, here we are after further investigation.
>
> - xbzrle: we don't need anything special on the reception side, we just
> decode inplace. So we are good. If we don't use xbzrle, we
> don't waste any resources either.
> - compression: We create the decompression threads always, with all its
> associated configuration.
>
>
> Why do I wanted to do this? For multifd, I don't really want to create
> multiple fd's except if configured, because I wait for the fd's to be
> "accepted" before continue, so I need a way to know if it is configured
> or no. So I need something like this.
>
> I started doing this for compression because when I was debugging I had
> too many threads waiting on qemu_cond_wait(), and I only wanted to look
> at the multifd ones.
>
> I had not througth that we didn't need to set capabilities on
> destination.
>
> Suggestions?
It's fine for new features to require it; newer libvirt already does
it for postcopy and does:
-incoming defer
sets the capabilities and parameters
migrate_incoming URI
However, enforcing it for old features sounds something we can't do.
Dave
> Later, Juan.
>
> >
> > Dave
> >
> >> Signed-off-by: Juan Quintela <address@hidden>
> >> ---
> >> migration/ram.c | 23 ++++++++++++++++++++++-
> >> 1 file changed, 22 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/migration/ram.c b/migration/ram.c
> >> index fb9252d..4bb707c 100644
> >> --- a/migration/ram.c
> >> +++ b/migration/ram.c
> >> @@ -2464,7 +2464,7 @@ static int ram_load_postcopy(QEMUFile *f)
> >>
> >> static int ram_load(QEMUFile *f, void *opaque, int version_id)
> >> {
> >> - int flags = 0, ret = 0;
> >> + int flags = 0, ret = 0, invalid_flags;
> >> static uint64_t seq_iter;
> >> int len = 0;
> >> /*
> >> @@ -2479,6 +2479,15 @@ static int ram_load(QEMUFile *f, void *opaque, int
> >> version_id)
> >> ret = -EINVAL;
> >> }
> >>
> >> + invalid_flags = 0;
> >> +
> >> + if (!migrate_use_xbzrle()) {
> >> + invalid_flags |= RAM_SAVE_FLAG_XBZRLE;
> >> + }
> >> +
> >> + if (!migrate_use_compression()) {
> >> + invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE;
> >> + }
> >> /* This RCU critical section can be very long running.
> >> * When RCU reclaims in the code start to become numerous,
> >> * it will be necessary to reduce the granularity of this
> >> @@ -2499,6 +2508,18 @@ static int ram_load(QEMUFile *f, void *opaque, int
> >> version_id)
> >> flags = addr & ~TARGET_PAGE_MASK;
> >> addr &= TARGET_PAGE_MASK;
> >>
> >> + if (flags & invalid_flags) {
> >> + if (flags & invalid_flags & RAM_SAVE_FLAG_XBZRLE) {
> >> + error_report("Received an unexpected XBRLE page");
> >> + }
> >> + if (flags & invalid_flags & RAM_SAVE_FLAG_COMPRESS_PAGE) {
> >> + error_report("Received an unexpected compressed page");
> >> + }
> >> +
> >> + ret = -EINVAL;
> >> + break;
> >> + }
> >> +
> >> if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
> >> RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE))
> >> {
> >> RAMBlock *block = ram_block_from_stream(f, flags);
> >> --
> >> 2.7.4
> >>
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK