From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48408) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4rEa-0004Cq-C5 for qemu-devel@nongnu.org; Fri, 24 Aug 2012 06:35:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4rEV-0007Jv-Om for qemu-devel@nongnu.org; Fri, 24 Aug 2012 06:35:24 -0400 Received: from smtp.eu.citrix.com ([62.200.22.115]:63052) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4rEV-0007I6-Hs for qemu-devel@nongnu.org; Fri, 24 Aug 2012 06:35:19 -0400 Message-ID: <1345804516.12501.182.camel@zakaz.uk.xensource.com> From: Ian Campbell Date: Fri, 24 Aug 2012 11:35:16 +0100 In-Reply-To: <50375721.7080503@citrix.com> References: <1345728471.12501.90.camel@zakaz.uk.xensource.com> <503680C5.6070509@citrix.com> <1345751525.23624.58.camel@dagon.hellion.org.uk> <50375721.7080503@citrix.com> Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Xen-devel] [XEN][RFC PATCH V2 11/17] xc: modify save/restore to support multiple device models List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Julien Grall Cc: "christian.limpach@gmail.com" , "xen-devel@lists.xen.org" , "qemu-devel@nongnu.org" , Stefano Stabellini On Fri, 2012-08-24 at 11:27 +0100, Julien Grall wrote: > On 08/23/2012 08:52 PM, Ian Campbell wrote: > > On Thu, 2012-08-23 at 20:13 +0100, Julien Grall wrote: > > > >> On 08/23/2012 02:27 PM, Ian Campbell wrote: > >> > >>> > >>>> @@ -103,6 +103,9 @@ static ssize_t rdexact(xc_interface *xch, struct restore_ctx *ctx, > >>>> #else > >>>> #define RDEXACT read_exact > >>>> #endif > >>>> + > >>>> +#define QEMUSIG_SIZE 21 > >>>> + > >>>> /* > >>>> ** In the state file (or during transfer), all page-table pages are > >>>> ** converted into a 'canonical' form where references to actual mfns > >>>> @@ -467,7 +522,7 @@ static int buffer_tail_hvm(xc_interface *xch, struct restore_ctx *ctx, > >>>> int vcpuextstate, uint32_t vcpuextstate_size) > >>>> { > >>>> uint8_t *tmp; > >>>> - unsigned char qemusig[21]; > >>>> + unsigned char qemusig[QEMUSIG_SIZE + 1]; > >>>> > >>>> > >>> An extra + 1 here? > >>> > >>> > >> QEMUSIG_SIZE doesn't take into account the '\0'. So we need to add 1. > >> If an error occurred, without +1, the output log lost the last character. > >> > > So this is just a bug fix for a pre-existing issue? > > > Yes. Can we get it as a separate change? > > >>> [...] > >>> > >>> > >>>> - qemusig[20] = '\0'; > >>>> + qemusig[QEMUSIG_SIZE] = '\0'; > >>>> > >>>> > >>> This is one bigger than it used to be now. > >>> > >>> Perhaps this is an unrelated bug fix (I haven't check the real length of > >>> the sig), in which case please can you split it out and submit > >>> separately? > >>> > >>> > >> #define QEMU_SIGNATURE "DeviceModelRecord0002" > >> Just checked, the length seems to be 21. I will send a patch with > >> this change. > >> > > Perhaps use either sizeof(QEMU_SIGNATURE) or strlen(QEMU_SIGNATURE) > > (depending on which semantics you want)? > > > Here, QEMU_SIZE needs to be define as strlen (QEMU_SIGNATURE), > but QEMU_SIGNATURE is not defined in libxc. It's defined > in libxl/libxl_internal.h. Oh, right, this again :-/ > By the way, I'm wondering why QEMU save (libxl__domain_save_device_model) > is made in libxl and restore (dump_qemu) in libxc ? Mostly historical accident, we'd really like to sort this out one way or the other but untangling the protocol and the callbacks etc is a pretty big job. In the meantime perhaps libxc could provide a suitable "typedef char device_model_signature_t[21]"? Ian.