On Fri, Jun 17, 2011 at 12:46 PM, Shriram Rajagopalan
<rshriram@xxxxxxxxx> wrote:
> On Fri, Jun 17, 2011 at 5:35 AM, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> wrote:
>>
>> On Thu, Jun 16, 2011 at 4:50 PM, Shriram Rajagopalan <rshriram@xxxxxxxxx>
>> wrote:
>> > @@ -846,6 +881,9 @@
>> > if (!countpages)
>> > return count;
>> >
>> > + if (buf->compression)
>> > + return pagebuf_get_one(xch, ctx, buf, fd, dom);
>>
>> I'm having trouble understanding the logic of this one. What's
>> supposed to be going on here?
>>
> The original xc_domain_restore code expects the list of pages immediately
> after the list of pfns for a batch.
>
> With compression, the compressed chunks are attached at the fag end of the
> pagebuf,
> [<pfn list>]+,[XC_SAVE_ID_* stuff like vcpu info, hvm_ident_pt, etc],
> [XC_SAVE_ID_COMPRESSED_DATA,compressed chunk]+
>
> So that if block diverts the pagebuf_get_one()'s control flow to
> consume the rest of pagebuf in usual recursive manner.
>
> original flow:
>
> pagebuf_get_one()
> read integer
> if integer is positive, its a count of number of pfns in the pfn list,
> read count*sizeof(pfn) from io_fd
> 1.1 parse pfn list and determine number of valid pages (countpages)
> 1.2. read (countpages * PAGE_SIZE) from io_fd
> -- basically read batch of pfns + pages
> else if its negative
> check for one of XC_SAVE_ID_* elements and read appropriate size from
> io_fd
> return pagebuf_get_one()
>
> **********
> The modified flow:
>
> pagebuf_get_one()
> read integer
> if integer is positive, its a count of number of pfns in the pfn list,
> read count*sizeof(pfn) from io_fd
> 1.1 parse pfn list and determine number of valid pages (countpages)
>>>1.2 if (compression) return pagebuf_get_one()
> --basically accumulate all the pfns and the (compressed) pages would
> come later
> else if its negative
> check for one of XC_SAVE_ID_* elements and read appropriate size from
> io_fd
> includes XC_SAVE_ID_COMPRESSED_DATA.
> return pagebuf_get_one()
OK -- you should add a comment there explaining that the data will
come later in an XC_SAVE_ID_COMPRESSED (or whatever) packet.
> oh shux. you are right. thanks. I could probably send this as
> XC_SAVE_ID_COMPRESSION
> after the last iteration and enable pagebuf->compression once its received.
Yeah, I think that should work -- the first round you'll be sending
uncompressed pages anyway.
Hmm -- while you're at it, could you add a comment somewhere in this
area saying that this is part of the wire protocol that can't be
changed without breaking backwards compatibility? Just to make it
easier to avoid this kind of error in the future...
Other than that, looks all right to me.
(Keir / IanJ, let me know if you want me to take a close enough look
to give a proper ACK.)
Thanks,
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home