Re: [Qemu-devel] [PATCH v0 0/7] Background snapshots

[Peter Xu]

On Fri, Jun 29, 2018 at 12:53:59PM +0100, Dr. David Alan Gilbert wrote:
> * Denis Plotnikov (address@hidden) wrote:
> > The patch set adds the ability to make external snapshots while VM is 
> > running.
> 
> cc'ing in Andrea since this uses sigsegv's to avoid userfault-wp that
> isn't there yet.
> 
> Hi Denis,
>   How robust are you finding this SEGV based trick; for example what
> about things like the kernel walking vhost queues or similar kernel
> nasties?

(I'm commenting on this old series to keep the discussion together)

If we want to make this series really work for people, we should
possibly need to know whether it could work with vhost (otherwise we
might need to go back to userfaultfd write-protection).

I digged a bit on the vhost-net IO, it should be using two ways to
write to guest memory:

- copy_to_user(): this should possibly still be able to be captured by
  mprotect() (after some confirmation from Paolo, but still we'd
  better try it out)

- kmap_atomic(): this is used to log dirty pages, this seems to be
  incompatible with mprotect() but I think we can just make sure dirty
  page tracking is disabled when live snapshot is enabled (please
  refer to the code clip below as a referece)

So IMHO this series should work with vhost if with logging off, but we
need to confirm...  Denis, do you want to do some more test with vhost
when you do your next post (possibly with changes like below)?

=====================

diff --git a/migration/ram.c b/migration/ram.c
index 24dea2730c..a3fa256143 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1605,6 +1605,10 @@ static void migration_bitmap_sync(RAMState *rs)
     int64_t end_time;
     uint64_t bytes_xfer_now;
 
+    if (live_snapshot) {
+        return;
+    }
+
     ram_counters.dirty_sync_count++;
 
     if (!rs->time_last_bitmap_sync) {
@@ -2952,7 +2956,10 @@ static void ram_init_bitmaps(RAMState *rs)
     rcu_read_lock();
 
     ram_list_init_bitmaps();
-    memory_global_dirty_log_start();
+
+    if (!live_snapshot) {
+        memory_global_dirty_log_start();
+    }
     migration_bitmap_sync(rs);
 
     rcu_read_unlock();


> 
> Dave
> 
> > The workflow to make a snapshot is the following:
> > 1. Pause the vm
> > 2. Make a snapshot of block devices using the scheme of your choice
> > 3. Turn on background-snapshot migration capability
> > 4. Start the migration using the destination (migration stream) of your 
> > choice.
> >    The migration will resume the vm execution by itself
> >    when it has the devices' states saved  and is ready to start ram writing
> >    to the migration stream.
> > 5. Listen to the migration finish event
> > 
> > The feature relies on KVM unapplied ability to report the faulting address.
> > Please find the KVM patch snippet to make the patchset work below:
> > 
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -XXXX,X +XXXX,XX @@ static int handle_ept_violation(struct kvm_vcpu 
> > *vcpu)
> >  
> >         vcpu->arch.exit_qualification = exit_qualification;
> >  
> > -       return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> > +       r = kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> > +        if (r == -EFAULT) {
> > +               unsigned long hva = kvm_vcpu_gfn_to_hva(vcpu, gpa >> 
> > PAGE_SHIFT);
> > +
> > +               vcpu->run->exit_reason = KVM_EXIT_FAIL_MEM_ACCESS;
> > +               vcpu->run->hw.hardware_exit_reason = 
> > EXIT_REASON_EPT_VIOLATION;
> > +               vcpu->run->fail_mem_access.hva = hva | (gpa & 
> > (PAGE_SIZE-1));
> > +               r = 0;
> > +
> > +       }
> > +       return r;
> > 
> > The patch to KVM can be sent if the patch set approved
> > 
> > Denis Plotnikov (7):
> >   migration: add background snapshot capability
> >   bitops: add some atomic versions of bitmap operations
> >   threads: add infrastructure to process sigsegv
> >   migration: add background snapshot infrastructure
> >   kvm: add failed memeory access exit reason
> >   kvm: add vCPU failed memeory access processing
> >   migration: add background snapshotting
> > 
> >  include/exec/ram_addr.h   |   7 +
> >  include/exec/ramlist.h    |   4 +-
> >  include/qemu/bitops.h     |  24 +++
> >  include/qemu/thread.h     |   5 +
> >  linux-headers/linux/kvm.h |   5 +
> >  migration/migration.c     | 141 +++++++++++++++-
> >  migration/migration.h     |   1 +
> >  migration/ram.c           | 333 ++++++++++++++++++++++++++++++++++++--
> >  migration/ram.h           |  11 +-
> >  migration/savevm.c        |  91 ++++++-----
> >  migration/savevm.h        |   2 +
> >  qapi/migration.json       |   6 +-
> >  target/i386/kvm.c         |  18 +++
> >  util/qemu-thread-posix.c  |  50 ++++++
> >  14 files changed, 635 insertions(+), 63 deletions(-)
> > 
> > -- 
> > 2.17.0
> > 
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
> 

Regards,

-- 
Peter Xu