From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48712) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjnZm-0002ps-Da for qemu-devel@nongnu.org; Mon, 12 May 2014 06:35:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WjnZg-0005PR-VE for qemu-devel@nongnu.org; Mon, 12 May 2014 06:35:18 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:34635 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjnZg-0005PD-La for qemu-devel@nongnu.org; Mon, 12 May 2014 06:35:12 -0400 Message-ID: <5370A3D4.5040900@kamp.de> Date: Mon, 12 May 2014 12:35:00 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1399717549-10961-1-git-send-email-pl@kamp.de> <87lhu78fkw.fsf@elfo.mitica> <5370A193.7060309@kamp.de> In-Reply-To: <5370A193.7060309@kamp.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] migration: catch unknown flags in ram_load List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com Cc: pbonzini@redhat.com, qemu-stable@nongnu.org, qemu-devel@nongnu.org, dgilbert@redhat.com Am 12.05.2014 12:25, schrieb Peter Lieven: > Am 12.05.2014 12:19, schrieb Juan Quintela: >> Peter Lieven wrote: >>> if a saved vm has unknown flags in the memory data qemu >>> currently simply ignores this flag and continues which >>> yields in an unpredictable result. >>> >>> this patch catches all unknown flags and >>> aborts the loading of the vm. >>> >>> CC: qemu-stable@nongnu.org >>> Signed-off-by: Peter Lieven >> ..... >> >> Once here, shouldn't be better to do this as: >> >> change do {} while () for while (true) {} >> >>> >>> @@ -1121,6 +1119,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) >>> } >>> } else if (flags & RAM_SAVE_FLAG_HOOK) { >>> ram_control_load_hook(f, flags); >>> + } else if (!(flags & RAM_SAVE_FLAG_EOS)) { >>> + ret = -EINVAL; >>> + goto done; >>> } >>> error = qemu_file_get_error(f); >>> if (error) { >> } else if (flags & RAM_SAVE_FLAG_HOOK) { >> ram_control_load_hook(f, flags); >> + } else if (flags & RAM_SAVE_FLAG_EOS) { >> + break; >> + } else { >> + ret = -EINVAL; >> + goto done; >> } >> error = qemu_file_get_error(f); >> if (error) { we can also drop the error variable I think and change the loop to while (!ret) {} >> } >> >> >> This way, we are checking RAM_SAVE_FLAG_EOS the same way than any other >> flag? And we don't have to duplicate the FLAG_NAME? > Ok, I will send a v2. > >> Unrelated to this patch, all the flags are a bitmap, but really, the >> ones that can be together are RAM_SAVE_FLAG_CONTINUE and the rest, all >> the others need to be alone. I am telling this because we have used >> already 8 flags, and we are using the low bits of offset to save the >> flags, we have 10 flags? Perhaps changing the last flag to mean that >> the low bits pass to be a counter? > Some better encoding would indeed be useful. I already thought > that we might run out of flags soon. We have 11 flags I think, > but there is not much space left. Reserving the last flag to indicate > that the lower 10 bits a are counter might be a good option. > > Peter > >> PD. No, I haven't investigated right now how RAM_SAVE_FLAG_HOOK works >> with all of this. >> >> Later, Juan. >>