[Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Laszlo Ersek]

Libvirt is growing support for x86_64 OVMF guests:

http://www.redhat.com/archives/libvir-list/2014-August/msg01045.html

An important feature of such guests is the persistent store for
non-volatile UEFI variables. This is implemented with if=pflash drives.
The referenced libvirt patchset sets up the varstore files for
single-host use.

Wrt. migration, two choices have been considered:
(a) full-blown live storage migration for the drives backing pflash
    devices,
(b) vs. a shortcut that exploits the special nature of pflash drives
    (namely, their minuscule size, and a RAMBlock that keeps the full
    contents of each pflash drive visible to the guest, and is
    up-to-date, at all times.)

Patch 1/2 is a trivial cleanup (some DPRINTF() calls in pflash_cfi01
have bit-rotted). Patch 2/2 seeks to implement choice (b), which is what
the libvirt patchset relies on for migration.

Thanks,
Laszlo

Laszlo Ersek (2):
  pflash_cfi01: fixup stale DPRINTF() calls
  pflash_cfi01: write flash contents to bdrv on incoming migration

 hw/block/pflash_cfi01.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] pflash_cfi01: fixup stale DPRINTF() calls

[Laszlo Ersek]

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/block/pflash_cfi01.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 2238f39..fddef39 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -209,11 +209,11 @@ static uint32_t pflash_devid_query(pflash_t *pfl, hwaddr offset)
     switch (boff & 0xFF) {
     case 0:
         resp = pfl->ident0;
-        DPRINTF("%s: Manufacturer Code %04x\n", __func__, ret);
+        DPRINTF("%s: Manufacturer Code %04x\n", __func__, resp);
         break;
     case 1:
         resp = pfl->ident1;
-        DPRINTF("%s: Device ID Code %04x\n", __func__, ret);
+        DPRINTF("%s: Device ID Code %04x\n", __func__, resp);
         break;
     default:
         DPRINTF("%s: Read Device Information offset=%x\n", __func__,
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] pflash_cfi01: write flash contents to bdrv on incoming migration

[Laszlo Ersek]

A drive that backs a pflash device is special:
- it is very small,
- its entire contents are kept in a RAMBlock at all times, covering the
  guest-phys address range that provides the guest's view of the emulated
  flash chip.

The pflash device model keeps the drive (the host-side file) and the
guest-visible flash contents in sync. When migrating the guest, the
guest-visible flash contents (the RAMBlock) is migrated by default, but on
the target host, the drive (the host-side file) remains in full sync with
the RAMBlock only if:
- the source and target hosts share the storage underlying the pflash
  drive,
- or the migration requests full or incremental block migration too, which
  then covers all drives.

Due to the special nature of pflash drives, the following scenario makes
sense as well:
- no full nor incremental block migration, covering all drives, alongside
  the base migration (justified eg. by shared storage for "normal" (big)
  drives),
- non-shared storage for pflash drives.

In this case, currently only those portions of the flash drive are updated
on the target disk that the guest reprograms while running on the target
host.

In order to restore accord, dump the entire flash contents to the bdrv in
a post_load() callback.

- The read-only check follows the other call-sites of pflash_update();
- both "pfl->ro" and pflash_update() reflect / consider the case when
  "pfl->bs" is NULL;
- the total size of the flash device is calculated as in
  pflash_cfi01_realize().

When using shared storage, or requesting full or incremental block
migration along with the normal migration, the patch should incur a
harmless rewrite from the target side.

It is assumed that, on the target host, RAM is loaded ahead of the call to
pflash_post_load().

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/block/pflash_cfi01.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index fddef39..593fbc5 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -94,10 +94,13 @@ struct pflash_t {
     void *storage;
 };
 
+static int pflash_post_load(void *opaque, int version_id);
+
 static const VMStateDescription vmstate_pflash = {
     .name = "pflash_cfi01",
     .version_id = 1,
     .minimum_version_id = 1,
+    .post_load = pflash_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(wcycle, pflash_t),
         VMSTATE_UINT8(cmd, pflash_t),
@@ -982,3 +985,14 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
 {
     return &fl->mem;
 }
+
+static int pflash_post_load(void *opaque, int version_id)
+{
+    pflash_t *pfl = opaque;
+
+    if (!pfl->ro) {
+        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
+        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
+    }
+    return 0;
+}
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Paolo Bonzini]

Il 23/08/2014 12:19, Laszlo Ersek ha scritto:
> Libvirt is growing support for x86_64 OVMF guests:
> 
> http://www.redhat.com/archives/libvir-list/2014-August/msg01045.html
> 
> An important feature of such guests is the persistent store for
> non-volatile UEFI variables. This is implemented with if=pflash drives.
> The referenced libvirt patchset sets up the varstore files for
> single-host use.
> 
> Wrt. migration, two choices have been considered:
> (a) full-blown live storage migration for the drives backing pflash
>     devices,
> (b) vs. a shortcut that exploits the special nature of pflash drives
>     (namely, their minuscule size, and a RAMBlock that keeps the full
>     contents of each pflash drive visible to the guest, and is
>     up-to-date, at all times.)
> 
> Patch 1/2 is a trivial cleanup (some DPRINTF() calls in pflash_cfi01
> have bit-rotted). Patch 2/2 seeks to implement choice (b), which is what
> the libvirt patchset relies on for migration.
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (2):
>   pflash_cfi01: fixup stale DPRINTF() calls
>   pflash_cfi01: write flash contents to bdrv on incoming migration
> 
>  hw/block/pflash_cfi01.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Alexey/David, I think hw/nvram/spapr_nvram.c should do the same.  It
doesn't have a vmstate, but you can probably use
qemu_add_vm_change_state_handler to the same effect.

Paolo

Re: [Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Daniel P. Berrange]

On Sat, Aug 23, 2014 at 12:19:05PM +0200, Laszlo Ersek wrote:
> Libvirt is growing support for x86_64 OVMF guests:
> 
> http://www.redhat.com/archives/libvir-list/2014-August/msg01045.html
> 
> An important feature of such guests is the persistent store for
> non-volatile UEFI variables. This is implemented with if=pflash drives.
> The referenced libvirt patchset sets up the varstore files for
> single-host use.
> 
> Wrt. migration, two choices have been considered:
> (a) full-blown live storage migration for the drives backing pflash
>     devices,
> (b) vs. a shortcut that exploits the special nature of pflash drives
>     (namely, their minuscule size, and a RAMBlock that keeps the full
>     contents of each pflash drive visible to the guest, and is
>     up-to-date, at all times.)

So, IIUC, with option b), libvirt will merely need to make sure that
the NVRAM var store file exists with the right size. QEMU will then
just 'do the right thing' for migration copying across the contents ?
If so that sounds nice to me.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Laszlo Ersek]

On 08/27/14 10:58, Daniel P. Berrange wrote:
> On Sat, Aug 23, 2014 at 12:19:05PM +0200, Laszlo Ersek wrote:
>> Libvirt is growing support for x86_64 OVMF guests:
>>
>> http://www.redhat.com/archives/libvir-list/2014-August/msg01045.html
>>
>> An important feature of such guests is the persistent store for
>> non-volatile UEFI variables. This is implemented with if=pflash drives.
>> The referenced libvirt patchset sets up the varstore files for
>> single-host use.
>>
>> Wrt. migration, two choices have been considered:
>> (a) full-blown live storage migration for the drives backing pflash
>>     devices,
>> (b) vs. a shortcut that exploits the special nature of pflash drives
>>     (namely, their minuscule size, and a RAMBlock that keeps the full
>>     contents of each pflash drive visible to the guest, and is
>>     up-to-date, at all times.)
> 
> So, IIUC, with option b), libvirt will merely need to make sure that
> the NVRAM var store file exists with the right size. QEMU will then
> just 'do the right thing' for migration copying across the contents ?
> If so that sounds nice to me.

Yes, that is the case.

Michal's patchset for libvirt pre-creates the empty varstore on the
incoming side, from the template file. That code path is shared with the
case when the VM starts up as a non-incoming one, and just has no
private varstore yet. The varstore created (from the template) is
"empty" in the PI / UEFI sense, but it's not all zeroes; it has some
internal structure.

Instantiating the VM-private varstore from the template is
- required for both size *and* contents reasons for a non-incoming
  startup, and
- required for size reasons *only* for an incoming startup.

http://www.redhat.com/archives/libvir-list/2014-August/msg01048.html

+    /* If the nvram path is configured already, there's nothing
+     * we need to do. Unless we are starting the destination side
+     * of migration in which case nvram is configured in the
+     * domain XML but the file doesn't exist yet. Moreover, after
+     * the migration is completed, qemu will invoke a
+     * synchronization write into the nvram file so we don't have
+     * to take care about transmitting the real data on the other
+     * side. */
+    if (loader->nvram && !migrated)
+        return 0;

Once the pflash_post_load() hook is called in the incoming qemu process,
the VM-private varstore (which libvirt has prepared from the template)
will be completely overwritten by QEMU. But the size of the file is
important information, for starting up the incoming qemu side.

(Any mismatch in size would be caught when the backing RAMBlock's size
is verified during migration.)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

On Sat, Aug 23, 2014 at 12:19:05PM +0200, Laszlo Ersek wrote:
> Libvirt is growing support for x86_64 OVMF guests:
> 
> http://www.redhat.com/archives/libvir-list/2014-August/msg01045.html
> 
> An important feature of such guests is the persistent store for
> non-volatile UEFI variables. This is implemented with if=pflash drives.
> The referenced libvirt patchset sets up the varstore files for
> single-host use.
> 
> Wrt. migration, two choices have been considered:
> (a) full-blown live storage migration for the drives backing pflash
>     devices,
> (b) vs. a shortcut that exploits the special nature of pflash drives
>     (namely, their minuscule size, and a RAMBlock that keeps the full
>     contents of each pflash drive visible to the guest, and is
>     up-to-date, at all times.)
> 
> Patch 1/2 is a trivial cleanup (some DPRINTF() calls in pflash_cfi01
> have bit-rotted). Patch 2/2 seeks to implement choice (b), which is what
> the libvirt patchset relies on for migration.
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (2):
>   pflash_cfi01: fixup stale DPRINTF() calls
>   pflash_cfi01: write flash contents to bdrv on incoming migration
> 
>  hw/block/pflash_cfi01.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Alexey Kardashevskiy]

On 08/25/2014 08:33 PM, Paolo Bonzini wrote:
> Il 23/08/2014 12:19, Laszlo Ersek ha scritto:
>> Libvirt is growing support for x86_64 OVMF guests:
>>
>> http://www.redhat.com/archives/libvir-list/2014-August/msg01045.html
>>
>> An important feature of such guests is the persistent store for
>> non-volatile UEFI variables. This is implemented with if=pflash drives.
>> The referenced libvirt patchset sets up the varstore files for
>> single-host use.
>>
>> Wrt. migration, two choices have been considered:
>> (a) full-blown live storage migration for the drives backing pflash
>>     devices,
>> (b) vs. a shortcut that exploits the special nature of pflash drives
>>     (namely, their minuscule size, and a RAMBlock that keeps the full
>>     contents of each pflash drive visible to the guest, and is
>>     up-to-date, at all times.)
>>
>> Patch 1/2 is a trivial cleanup (some DPRINTF() calls in pflash_cfi01
>> have bit-rotted). Patch 2/2 seeks to implement choice (b), which is what
>> the libvirt patchset relies on for migration.
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (2):
>>   pflash_cfi01: fixup stale DPRINTF() calls
>>   pflash_cfi01: write flash contents to bdrv on incoming migration
>>
>>  hw/block/pflash_cfi01.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Alexey/David, I think hw/nvram/spapr_nvram.c should do the same.  It
> doesn't have a vmstate, but you can probably use
> qemu_add_vm_change_state_handler to the same effect.

I am not sure I understood the proposal correctly.

Right now we use NVRAM on sPAPR as:
-drive id=id3,if=none,file=qemu_nvram.img
-global spapr-nvram.drive=id3

So the NVRAM file is BlockDriverState and HMP's "migrate -b" copies the
content just fine.

What is missing here? Thanks.


-- 
Alexey

Re: [Qemu-devel] [PATCH 0/2] pflash (UEFI varstore) migration shortcut for libvirt

[Paolo Bonzini]

Il 19/09/2014 08:48, Alexey Kardashevskiy ha scritto:
> Right now we use NVRAM on sPAPR as:
> -drive id=id3,if=none,file=qemu_nvram.img
> -global spapr-nvram.drive=id3
> 
> So the NVRAM file is BlockDriverState and HMP's "migrate -b" copies the
> content just fine.
> 
> What is missing here? Thanks.

"migrate -b" is a big hammer, because it transfers all disks.  In some
cases it is useful to have shared storage for disks and non-shared
storage for variable stores.

For UEFI, we are using RAM migration to transfer non-volatile RAM from
the source to the destination.  This is done by loading the whole
contents of nvram into a RAM MemoryRegion at VM startup, and storing it
at postload time.  The latter is done with this patch.

Paolo