Re: [Qemu-devel] KVM call agenda for tuesday 31

[Peter Maydell]

On 9 February 2012 22:23, Peter Maydell <address@hidden> wrote:
> Ping re the VMState and variable sized arrays issue. I don't
> see any consensus in this discussion for a different approach,
> so should we just commit Mitsyanko's patchset?

>From an IRC conversation I just had with Anthony and Juan:
===begin==
14:51 < pm215> quintela: do you have an opinion on the vmstate variable-
length array stuff (needed for sd card) ?
14:51 < quintela> pm havent looked, email title?
14:52 < pm215> "KVM call agenda for tuesday 31" is the most recent
discussion :-)
14:53 < pm215> http://patchwork.ozlabs.org/patch/137732/ and
 http://patchwork.ozlabs.org/patch/137733/ are the relevant patches
14:54 < quintela> pm215: found it, that it is a difficult thing to do (TM)
14:54 < quintela> it should be on the "card" file, or whatever :-(
14:55 < quintela> notice the "should" part.
14:55 < pm215> I'm not sure what you mean, can you elaborate?
14:57 < quintela> pm215: sect is number of sectors, right?
14:58 < pm215> yes
14:59 < quintela> so, a 1GB card would have around 8MB array?
14:59 < quintela> took or left some byties here andthere.
14:59 < quintela> bytes indeed.
14:59 < quintela> I _think_ that we should put that in a save_live
 section, but that is just me (TM)
15:00 < quintela> I guess that at some point, people are going to need
 bigger SD cards (16GB are already on the wild, right)
15:00 < quintela> and that would make live migration just impossible
15:00 < quintela> or very slow, that is completely equivalent.
15:01 < quintela> it is my understanding that AHCI is using similar code,
 or did I missread some of the information?
15:03 < pm215> I think alex would like ahci to use a similar 'variable
 length array' thing, but in that case the array is much smaller
15:03 < aliguori> pm215, it's large but of bounded size, right?
15:03 < aliguori> for SD cards?
15:03 < quintela> aliguori: number of sectors * 4 bytes
15:03 < quintela> aliguori: so hugeeeeeeeeeeeeeeeeee
15:04 < quintela> 8MB array for each 1GB of disk.
15:04 < quintela> but or take some bytes.
15:04 < aliguori> quintela, you cannot save that much data via savevm
15:04 < quintela> this is more than all other devices combined in a
 normal instalaltion.
15:04 < aliguori> that's too much
15:04 < quintela> aliguori: see my answer, we need a save_live section,
 really.
15:04 < aliguori> it will screw up the live migration downtime algorithm
15:04 < aliguori> pm215, ^
15:04 < aliguori> or just treat it as a ram section
15:05 < aliguori> qemu_ram_alloc() it, and call it a day
15:05 < pm215> the spec isn't very clear, but I think technically this
 info should go in the sd card image, except there's no way to tack
 additional info into a raw file
15:05 < aliguori> pm215, yeah, qemu_ram_alloc() is the way to go I think,
 that makes it effectively volatile on-card RAM
15:05 < quintela> pm215: I fully agree that it should go into the card
 image, but ..... no space for it :-(
15:05 < aliguori> which i think makes sense
15:05 < quintela> pm215: another thing, forgetting about migration at all.
15:06 < quintela> how does this work if you stop marchine and restart
 it again?
15:06  * quintela guess that it is stored somewhere?
15:06 < quintela> s/marchine/machine/
15:06 < pm215> no, we just assume that any fresh sd card image has no
 write-protect set up
15:07 < quintela> pm215: what is stored on that image? /me would have
 assumed that wearing information
15:07 < quintela> but that is without reading the whole code.
15:09 < quintela> humm, it looks like only 1 bit is used for each sector,
 why are we storing 32 bits if we only use 1 bit?
15:09 < pm215> it's write-protect : you can set parts of the sd card to
 not be writeable (with a granularity of a "write-protect group size")
15:09 < pm215> we probably don't implement fantastically efficiently
15:10 < quintela> pm215: ok, only 1 bit is needed.
15:10 < quintela> we can move to 1bit/sector (8 times smaller)
15:10 < quintela> but I still think that doing the qemu_ram_alloc()
 trick that aliguori pointed is the easiest way to fix it
15:11 < quintela> you can create a ram_save_live section, but it is
 going to be more complex for almost no gain
15:11 < pm215> ah, so we qemu_ram_alloc() it and then the contents get
 transferred in the same way as main memory ?
15:12 < pm215> ...that is in exec-obsolete.h and marked as "to be
 removed soon"...
15:13 < aliguori> pm215, yeah, so you'll need to create it using
 whatever the new fancy memory api interface is
15:13 < aliguori> pm215, note that whenever you touch that memory, you
 have to set the dirty bits appropriately
15:13 < aliguori> or else live migration won't work
15:14 < quintela> aliguori: if they have to touch the dirty bits, it is
 equivalent to do their own save_live section.
15:14 < quintela> but I agree that this is the only "easy" solution.
15:17 < pm215> doesn't sound too hard...
15:18 < quintela> as usual with vmstate, problem is testing (althought
 shouldn't be very difficult, either)
===endit===

Short summary:
 * switch wp groups to bitfield rather than int array
 * convert sd.c to use memory_region_init_ram() to allocate the wp groups
(being careful to use memory_region_set_dirty() when we touch them)
 * we don't need variable-length fields for sd.c any more
 * rest of the vmstate conversion is straightforward

-- PMM