[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: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] migration: Test for disabled features on reception |
Date: |
Tue, 15 Nov 2016 11:07:13 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
"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?
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