[PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Pawel Zmarzly]

Snapshots made with mapped-ram and x-ignore-shared flags are
not parsed properly.

Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
---
 migration/ram.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 29f016cb25..85fdc810ab 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4277,6 +4277,11 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
         id[len] = 0;
         length = qemu_get_be64(f);
 
+        if (migrate_ignore_shared()) {
+            /* Read and discard the x-ignore-shared memory region address */
+            qemu_get_be64(f);
+        }
+
         block = qemu_ram_block_by_name(id);
         if (block) {
             ret = parse_ramblock(f, block, length);
-- 
2.52.0

Re: [PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Peter Xu]

On Tue, Nov 25, 2025 at 05:46:49PM +0000, Pawel Zmarzly wrote:
> Snapshots made with mapped-ram and x-ignore-shared flags are
> not parsed properly.
> 
> Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
> ---
>  migration/ram.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 29f016cb25..85fdc810ab 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4277,6 +4277,11 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
>          id[len] = 0;
>          length = qemu_get_be64(f);
>  
> +        if (migrate_ignore_shared()) {
> +            /* Read and discard the x-ignore-shared memory region address */
> +            qemu_get_be64(f);
> +        }
> +
>          block = qemu_ram_block_by_name(id);
>          if (block) {
>              ret = parse_ramblock(f, block, length);
> -- 
> 2.52.0
> 

Thanks for the patch, though the u64 was parsed in parse_ramblock()
instead.  Would you consider refactoring that function instead?

-- 
Peter Xu

Re: [PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Nov 25, 2025 at 05:46:49PM +0000, Pawel Zmarzly wrote:
>> Snapshots made with mapped-ram and x-ignore-shared flags are
>> not parsed properly.
>> 
>> Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
>> ---
>>  migration/ram.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 29f016cb25..85fdc810ab 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -4277,6 +4277,11 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
>>          id[len] = 0;
>>          length = qemu_get_be64(f);
>>  
>> +        if (migrate_ignore_shared()) {
>> +            /* Read and discard the x-ignore-shared memory region address */
>> +            qemu_get_be64(f);
>> +        }
>> +
>>          block = qemu_ram_block_by_name(id);
>>          if (block) {
>>              ret = parse_ramblock(f, block, length);
>> -- 
>> 2.52.0
>> 
>
> Thanks for the patch, though the u64 was parsed in parse_ramblock()
> instead.  Would you consider refactoring that function instead?

There's actually not much going on in terms of "parsing" in
parse_ramblock(). I think we could move the migrate_ignore_shared() from
the end of the function to before the mapped-ram check().

Re: [PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Peter Xu]

On Tue, Nov 25, 2025 at 06:40:12PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Nov 25, 2025 at 05:46:49PM +0000, Pawel Zmarzly wrote:
> >> Snapshots made with mapped-ram and x-ignore-shared flags are
> >> not parsed properly.
> >> 
> >> Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
> >> ---
> >>  migration/ram.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >> 
> >> diff --git a/migration/ram.c b/migration/ram.c
> >> index 29f016cb25..85fdc810ab 100644
> >> --- a/migration/ram.c
> >> +++ b/migration/ram.c
> >> @@ -4277,6 +4277,11 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
> >>          id[len] = 0;
> >>          length = qemu_get_be64(f);
> >>  
> >> +        if (migrate_ignore_shared()) {
> >> +            /* Read and discard the x-ignore-shared memory region address */
> >> +            qemu_get_be64(f);
> >> +        }
> >> +
> >>          block = qemu_ram_block_by_name(id);
> >>          if (block) {
> >>              ret = parse_ramblock(f, block, length);
> >> -- 
> >> 2.52.0
> >> 
> >
> > Thanks for the patch, though the u64 was parsed in parse_ramblock()
> > instead.  Would you consider refactoring that function instead?
> 
> There's actually not much going on in terms of "parsing" in
> parse_ramblock(). I think we could move the migrate_ignore_shared() from
> the end of the function to before the mapped-ram check().

Yes, that's also what I meant if it wasn't clear.. it was parsed into a
hwaddr, and it was used to verify the addresses match.

If that check is needed for ignore-shared blocks, then these checks should
also apply when mapped-ram is enabled on top of whatever ramblock got
ignored during migration.

Since the discussion started, I am actually not sure if we do this all
right for two things..

(1) When mapped-ram is enabled, do we actually need to setup those
    ramblocks in mapped_ram_setup_ramblock()?

    That is, when a ramblock returns migrate_ram_is_ignored()==true, IIUC
    we don't need to allocate bitmap or page chunks for it?

    We likely don't need to change this easily, because this will change
    file format.. I'm also not sure if this is a major issue, logically
    when ignore-shared is used we normally shouldn't need mapped-ram.. vice
    versa.  So I may need to better understand the use case first on
    enabling the two..

(2) Is the check proper on validating mr->addr didn't change?

    This is a question on the check itself when ignore-shared enabled,
    with/without mapped-ram enabled.  That is, I question whether this
    check is useful or valid at all:
    
    if (migrate_ignore_shared()) {
        hwaddr addr = qemu_get_be64(f);
        if (migrate_ram_is_ignored(block) &&
            block->mr->addr != addr) {
            error_report("Mismatched GPAs for block %s "
                         "%" PRId64 "!= %" PRId64, block->idstr,
                         (uint64_t)addr, (uint64_t)block->mr->addr);
            return -EINVAL;
        }
    }

    In the error, it said "GPA", but mr->addr isn't GPA.. it's the offset
    of the MR within the MR's parent container MR..  So if the parent is
    the root MR / system_memory, then it is the GPA, however I don't see it
    guaranteed..

    My gut feeling is we almost always rely on proper QEMU cmdlines anyway
    to make migration work.  I wonder if we should just remove this check
    (in case it might break when mr's parent isn't the root MR).

This is irrelevant of this specific fix, so it doesn't need to block a
repost..

-- 
Peter Xu

Re: [PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Paweł Zmarzły]

On Tue, 25 Nov 2025 at 22:29, Peter Xu <peterx@redhat.com> wrote:
> On Tue, Nov 25, 2025 at 06:40:12PM -0300, Fabiano Rosas wrote:
> > Peter Xu <peterx@redhat.com> writes:
> >
> > > Thanks for the patch, though the u64 was parsed in parse_ramblock()
> > > instead.  Would you consider refactoring that function instead?

Refactored in V2.

> (1) When mapped-ram is enabled, do we actually need to setup those
>     ramblocks in mapped_ram_setup_ramblock()?
>
>     That is, when a ramblock returns migrate_ram_is_ignored()==true, IIUC
>     we don't need to allocate bitmap or page chunks for it?
>
>     We likely don't need to change this easily, because this will change
>     file format.. I'm also not sure if this is a major issue, logically
>     when ignore-shared is used we normally shouldn't need mapped-ram.. vice
>     versa.  So I may need to better understand the use case first on
>     enabling the two..

Re "when ignore-shared is used we normally shouldn't need mapped-ram" : since
memory-backend-file + numa only affect pc.ram and the rest of RAMBlocks are
serialized as usual, I can imagine someone may want to use mapped-ram format
for these remaining RAMBlocks for minor gains. This is hypothetical though, as
currently the memory-backend-file + numa + mapped-ram + ignore-shared
combination
causes QEMU to fail to write migration to disk:

    $ build/qemu-system-x86_64 \
        -m 1024 \
        -nographic \
        -object
memory-backend-file,id=ram0,size=1G,mem-path=/home/pzmarzly/repos/qemu-data/mem.img,align=1M,share=on
\
        -numa node,memdev=ram0
    (qemu) migrate_set_capability x-ignore-shared on
    (qemu) migrate_set_capability mapped-ram on
    (qemu) migrate -d file:/home/pzmarzly/repos/qemu-data/vm-shared.state
    (qemu) qemu-system-x86_64: Failed to write bitmap to file: Unable
to write to file: Bad address

Seems like another bug, I'll give it a shot.

> (2) Is the check proper on validating mr->addr didn't change?
>
>     This is a question on the check itself when ignore-shared enabled,
>     with/without mapped-ram enabled.  That is, I question whether this
>     check is useful or valid at all:
>
>     if (migrate_ignore_shared()) {
>         hwaddr addr = qemu_get_be64(f);
>         if (migrate_ram_is_ignored(block) &&
>             block->mr->addr != addr) {
>             error_report("Mismatched GPAs for block %s "
>                          "%" PRId64 "!= %" PRId64, block->idstr,
>                          (uint64_t)addr, (uint64_t)block->mr->addr);
>             return -EINVAL;
>         }
>     }
>
>     In the error, it said "GPA", but mr->addr isn't GPA.. it's the offset
>     of the MR within the MR's parent container MR..  So if the parent is
>     the root MR / system_memory, then it is the GPA, however I don't see it
>     guaranteed..

I only understand some of these words, having only started to dig into
QEMU codebase.
Feel free to change/remove this check as you wish.

Re: [PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Nov 25, 2025 at 06:40:12PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > On Tue, Nov 25, 2025 at 05:46:49PM +0000, Pawel Zmarzly wrote:
>> >> Snapshots made with mapped-ram and x-ignore-shared flags are
>> >> not parsed properly.
>> >> 
>> >> Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
>> >> ---
>> >>  migration/ram.c | 5 +++++
>> >>  1 file changed, 5 insertions(+)
>> >> 
>> >> diff --git a/migration/ram.c b/migration/ram.c
>> >> index 29f016cb25..85fdc810ab 100644
>> >> --- a/migration/ram.c
>> >> +++ b/migration/ram.c
>> >> @@ -4277,6 +4277,11 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
>> >>          id[len] = 0;
>> >>          length = qemu_get_be64(f);
>> >>  
>> >> +        if (migrate_ignore_shared()) {
>> >> +            /* Read and discard the x-ignore-shared memory region address */
>> >> +            qemu_get_be64(f);
>> >> +        }
>> >> +
>> >>          block = qemu_ram_block_by_name(id);
>> >>          if (block) {
>> >>              ret = parse_ramblock(f, block, length);
>> >> -- 
>> >> 2.52.0
>> >> 
>> >
>> > Thanks for the patch, though the u64 was parsed in parse_ramblock()
>> > instead.  Would you consider refactoring that function instead?
>> 
>> There's actually not much going on in terms of "parsing" in
>> parse_ramblock(). I think we could move the migrate_ignore_shared() from
>> the end of the function to before the mapped-ram check().
>
> Yes, that's also what I meant if it wasn't clear.. it was parsed into a
> hwaddr, and it was used to verify the addresses match.
>
> If that check is needed for ignore-shared blocks, then these checks should
> also apply when mapped-ram is enabled on top of whatever ramblock got
> ignored during migration.
>

Right, because ignore_shared implies putting the MR address in the
stream, but there is still the matter of whether the pages will actually
be read on the destination.

Should ram_save_setup() use RAMBLOCK_FOREACH_NOT_IGNORED instead of
RAMBLOCK_FOREACH_MIGRATABLE? I don't immediately see why not...

> Since the discussion started, I am actually not sure if we do this all
> right for two things..
>
> (1) When mapped-ram is enabled, do we actually need to setup those
>     ramblocks in mapped_ram_setup_ramblock()?
>
>     That is, when a ramblock returns migrate_ram_is_ignored()==true, IIUC
>     we don't need to allocate bitmap or page chunks for it?
>
>     We likely don't need to change this easily, because this will change
>     file format.. I'm also not sure if this is a major issue, logically
>     when ignore-shared is used we normally shouldn't need mapped-ram.. vice
>     versa.  So I may need to better understand the use case first on
>     enabling the two..
>

Thanks, Peter. A keen eye, as usual. I was searching for this reasoning
when looking at the code, but it missed me.

I looked back at the very first version of fixed-ram, which wasn't
authored by me and there is indeed no mention or expectation of handling
shared ram. So I think this is at this point unspecified.

What is the current impact of having those pages in? We're "just"
wasting cycles writing to the file, AFAICS. We'd better, at least,
sanitise that part to avoid the extra work.

> (2) Is the check proper on validating mr->addr didn't change?
>
>     This is a question on the check itself when ignore-shared enabled,
>     with/without mapped-ram enabled.  That is, I question whether this
>     check is useful or valid at all:
>     
>     if (migrate_ignore_shared()) {
>         hwaddr addr = qemu_get_be64(f);
>         if (migrate_ram_is_ignored(block) &&
>             block->mr->addr != addr) {
>             error_report("Mismatched GPAs for block %s "
>                          "%" PRId64 "!= %" PRId64, block->idstr,
>                          (uint64_t)addr, (uint64_t)block->mr->addr);
>             return -EINVAL;
>         }
>     }
>

I agree with all you say below, but I think an earlier question would
be: why put the address on the stream in the first place? Is this just
hardening of some sort?

The commit the introduces the feature has me wondering:

fbd162e629 ("migration: Add an ability to ignore shared RAM blocks")

  during save:
  
  +        if (migrate_ignore_shared()) {
  +            qemu_put_be64(f, block->mr->addr);
  +            qemu_put_byte(f, ramblock_is_ignored(block) ? 1 : 0);
  +        }

  during load:
  +    if (ramblock_is_ignored(block)) {
           error_report("block %s should not be migrated !", id);
           return NULL;
       }

If we know it's ignored, why send anything at all? (also, "to ignore"
has a meaning, we should stick to it)

>     In the error, it said "GPA", but mr->addr isn't GPA.. it's the offset
>     of the MR within the MR's parent container MR..  So if the parent is
>     the root MR / system_memory, then it is the GPA, however I don't see it
>     guaranteed..
>

Looking at the initial commit, I think this is all sanity check, maybe
to ensure some sort of stream compatibility. Or to make sure the stream
is stateful and we're not confusing an ignored block with an
(incorrectly) ignored one.

>     My gut feeling is we almost always rely on proper QEMU cmdlines anyway
>     to make migration work.  I wonder if we should just remove this check
>     (in case it might break when mr's parent isn't the root MR).
>

I believe it's ok to remove the check. I wish we had a compatible way to
remove handling of ignored blocks altogether, but I guess this u64 is
now on the stream forever?

> This is irrelevant of this specific fix, so it doesn't need to block a
> repost..

Re: [PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Paweł Zmarzły]

On Wed, 26 Nov 2025 at 12:45, Fabiano Rosas <farosas@suse.de> wrote:
>
> Should ram_save_setup() use RAMBLOCK_FOREACH_NOT_IGNORED instead of
> RAMBLOCK_FOREACH_MIGRATABLE? I don't immediately see why not...

I just submitted a new patch "migration: Fix writing mapped_ram +
ignore_shared snapshots" to skip ignored RAMBlocks. I think it may
still be useful to indicate their presence and record their sizes, so
I only skipped writing the contents, keeping the header. LMK what you
think.

Re: [PATCH] migration: fix parsing snapshots with x-ignore-shared flag

[Peter Xu]

On Wed, Nov 26, 2025 at 09:45:35AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Nov 25, 2025 at 06:40:12PM -0300, Fabiano Rosas wrote:
> >> Peter Xu <peterx@redhat.com> writes:
> >> 
> >> > On Tue, Nov 25, 2025 at 05:46:49PM +0000, Pawel Zmarzly wrote:
> >> >> Snapshots made with mapped-ram and x-ignore-shared flags are
> >> >> not parsed properly.
> >> >> 
> >> >> Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
> >> >> ---
> >> >>  migration/ram.c | 5 +++++
> >> >>  1 file changed, 5 insertions(+)
> >> >> 
> >> >> diff --git a/migration/ram.c b/migration/ram.c
> >> >> index 29f016cb25..85fdc810ab 100644
> >> >> --- a/migration/ram.c
> >> >> +++ b/migration/ram.c
> >> >> @@ -4277,6 +4277,11 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
> >> >>          id[len] = 0;
> >> >>          length = qemu_get_be64(f);
> >> >>  
> >> >> +        if (migrate_ignore_shared()) {
> >> >> +            /* Read and discard the x-ignore-shared memory region address */
> >> >> +            qemu_get_be64(f);
> >> >> +        }
> >> >> +
> >> >>          block = qemu_ram_block_by_name(id);
> >> >>          if (block) {
> >> >>              ret = parse_ramblock(f, block, length);
> >> >> -- 
> >> >> 2.52.0
> >> >> 
> >> >
> >> > Thanks for the patch, though the u64 was parsed in parse_ramblock()
> >> > instead.  Would you consider refactoring that function instead?
> >> 
> >> There's actually not much going on in terms of "parsing" in
> >> parse_ramblock(). I think we could move the migrate_ignore_shared() from
> >> the end of the function to before the mapped-ram check().
> >
> > Yes, that's also what I meant if it wasn't clear.. it was parsed into a
> > hwaddr, and it was used to verify the addresses match.
> >
> > If that check is needed for ignore-shared blocks, then these checks should
> > also apply when mapped-ram is enabled on top of whatever ramblock got
> > ignored during migration.
> >
> 
> Right, because ignore_shared implies putting the MR address in the
> stream, but there is still the matter of whether the pages will actually
> be read on the destination.
> 
> Should ram_save_setup() use RAMBLOCK_FOREACH_NOT_IGNORED instead of
> RAMBLOCK_FOREACH_MIGRATABLE? I don't immediately see why not...

Yes what you said makes sense, even if we may need to change more than
that..

Currently we'll dump the total amount of mem first before dumping the
ramblocks, that is this line:

        qemu_put_be64(f, ram_bytes_total_with_ignored()
                         | RAM_SAVE_FLAG_MEM_SIZE);

RAMBLOCK_FOREACH_MIGRATABLE() is used in ram_bytes_total_with_ignored(),
hence we need to keep using it for the loop later or the total size will
stop matching..

So if we change, then we'll at least also need to change there.  Either of
such change will be an ABI change.. that goes back to the "common migration
challenge" on compatibilities of old snapshot taken to be loadable on new
QEMUs..  Sad. :(

> 
> > Since the discussion started, I am actually not sure if we do this all
> > right for two things..
> >
> > (1) When mapped-ram is enabled, do we actually need to setup those
> >     ramblocks in mapped_ram_setup_ramblock()?
> >
> >     That is, when a ramblock returns migrate_ram_is_ignored()==true, IIUC
> >     we don't need to allocate bitmap or page chunks for it?
> >
> >     We likely don't need to change this easily, because this will change
> >     file format.. I'm also not sure if this is a major issue, logically
> >     when ignore-shared is used we normally shouldn't need mapped-ram.. vice
> >     versa.  So I may need to better understand the use case first on
> >     enabling the two..
> >
> 
> Thanks, Peter. A keen eye, as usual. I was searching for this reasoning
> when looking at the code, but it missed me.
> 
> I looked back at the very first version of fixed-ram, which wasn't
> authored by me and there is indeed no mention or expectation of handling
> shared ram. So I think this is at this point unspecified.
> 
> What is the current impact of having those pages in? We're "just"
> wasting cycles writing to the file, AFAICS. We'd better, at least,
> sanitise that part to avoid the extra work.

We do not write to those chunks.. See:

static void pss_find_next_dirty(PageSearchStatus *pss)
{
    RAMBlock *rb = pss->block;
    unsigned long size = rb->used_length >> TARGET_PAGE_BITS;
    unsigned long *bitmap = rb->bmap;

    if (migrate_ram_is_ignored(rb)) {
        /* Points directly to the end, so we know no dirty page */
        pss->page = size;
        return;
    }
    ...
}

So essentially those blobs (bitmap + page chunks) will be reserved but kept
as-is, afaict.  RAM iterator jumps over them.

Likely that's a good news, at least what you raised as concern isn't a
problem so far, it's only the image storage that will be wasted keeping
them zeros.

> 
> > (2) Is the check proper on validating mr->addr didn't change?
> >
> >     This is a question on the check itself when ignore-shared enabled,
> >     with/without mapped-ram enabled.  That is, I question whether this
> >     check is useful or valid at all:
> >     
> >     if (migrate_ignore_shared()) {
> >         hwaddr addr = qemu_get_be64(f);
> >         if (migrate_ram_is_ignored(block) &&
> >             block->mr->addr != addr) {
> >             error_report("Mismatched GPAs for block %s "
> >                          "%" PRId64 "!= %" PRId64, block->idstr,
> >                          (uint64_t)addr, (uint64_t)block->mr->addr);
> >             return -EINVAL;
> >         }
> >     }
> >
> 
> I agree with all you say below, but I think an earlier question would
> be: why put the address on the stream in the first place? Is this just
> hardening of some sort?

Yep; that's how I see it too, only that the checks are actually not
safe or at least causing more trouble than help..

Side note: not sure if you noticed, the 2nd check was removed here:

    commit b17fbbe55cba95ad323b27c2f9f3e22eb6364d4b
    Author: Peng Tao <tao.peng@linux.alibaba.com>
    Date:   Fri Jun 14 14:35:13 2019 +0800

    migration: allow private destination ram with x-ignore-shared

That commit was to enable some share=off memory loading from a share=on
memory snapshot use case, which is pretty interesting.

> 
> The commit the introduces the feature has me wondering:
> 
> fbd162e629 ("migration: Add an ability to ignore shared RAM blocks")
> 
>   during save:
>   
>   +        if (migrate_ignore_shared()) {
>   +            qemu_put_be64(f, block->mr->addr);
>   +            qemu_put_byte(f, ramblock_is_ignored(block) ? 1 : 0);
>   +        }
> 
>   during load:
>   +    if (ramblock_is_ignored(block)) {
>            error_report("block %s should not be migrated !", id);
>            return NULL;
>        }
> 
> If we know it's ignored, why send anything at all? (also, "to ignore"
> has a meaning, we should stick to it)

If we do not have the compatibility issue to consider, I agree with you
that all over ram.c we should only care about "not-ignored", rather than
"migratable".

Now we need to evaluate how bad it is with the current status quo, and
whether we will need a property enabling new streams doing the right thing.
So we can choose to keep this as-is, but introduce a flag only if necessary
and confident we should remove this overhead.

> 
> >     In the error, it said "GPA", but mr->addr isn't GPA.. it's the offset
> >     of the MR within the MR's parent container MR..  So if the parent is
> >     the root MR / system_memory, then it is the GPA, however I don't see it
> >     guaranteed..
> >
> 
> Looking at the initial commit, I think this is all sanity check, maybe
> to ensure some sort of stream compatibility. Or to make sure the stream
> is stateful and we're not confusing an ignored block with an
> (incorrectly) ignored one.
> 
> >     My gut feeling is we almost always rely on proper QEMU cmdlines anyway
> >     to make migration work.  I wonder if we should just remove this check
> >     (in case it might break when mr's parent isn't the root MR).
> >
> 
> I believe it's ok to remove the check. I wish we had a compatible way to
> remove handling of ignored blocks altogether, but I guess this u64 is
> now on the stream forever?

Yes. :(

> 
> > This is irrelevant of this specific fix, so it doesn't need to block a
> > repost..
> 

-- 
Peter Xu