Skip to content

L
libvirt

openeuler / libvirt

通知 3
Star 0
Fork 0
L
libvirt

体验新版 GitCode，发现更多精彩内容 >>

切换分支/标签
  1. 22 3月, 2019 8 次提交
    • E
      snapshot: Add accessor for reparenting snapshot children · ced0898f
      由 Eric Blake 提交于 
      Rather than allowing a leaky abstraction where multiple drivers have
to open-code operations that update the relations in a
virDomainSnapshotObjList, it is better to add accessor functions so
that updates to relations are maintained closer to the internals.
This patch starts the task with a single new function:
virDomainSnapshotMoveChildren(). The logic might not be immediately
obvious [okay, that's an understatement - the existing code uses black
magic ;-)], so here's an overview: The old code has an implicit for
loop around each call to qemuDomainSnapshotReparentChildren() by using
virDomainSnapshotForEachChild() (you'll need a wider context than
git's default of 3 lines to see that); the new code has a more visible
for loop. Then it helps if you realize that the code is making two
separate changes to each child object: STRDUP of the new parent name
prior to writing XML files (unchanged), and touching up the pointer to
the parent object (refactored); the end result is the same whether a
single pass made both changes (both in driver code), or whether it is
split into two passes making one change each (one in driver code, the
other in the new accessor).
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      ced0898f
    • E
      snapshot: Track current snapshot in virDomainSnapshotObjList · 4819f54b
      由 Eric Blake 提交于 
      It is easier to track the current snapshot as part of the list of
snapshots. In particular, doing so lets us guarantee that the current
snapshot is cleared if that snapshot is removed from the list (rather
than depending on the caller to do so, and risking a use-after-free
problem, such as the one recently patched in 1db9d0ef).  This
requires the addition of several new accessor functions, as well as a
useful return type for virDomainSnapshotObjListRemove().  A few error
handling sites that were previously setting vm->current_snapshot =
NULL can now be dropped, because the previous function call has now
done it already.  Also, qemuDomainRevertToSnapshot() was setting the
current vm twice, so keep only the one used on the success path.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      4819f54b
    • E
      snapshot: Rework parse logic during libvirt startup · 40bc98dd
      由 Eric Blake 提交于 
      Rework the logic in qemuDomainSnapshotLoad() to set
vm->current_snapshot only once at the end of the loop, rather than
repeatedly querying it during the loop, to make it easier for the next
patch to use accessor functions rather than direct manipulation of
vm->current_snapshot.  When encountering multiple snapshots claiming
to be current (based on the presence of an <active>1</active> element
in the XML, which libvirt only outputs for internal use and not for
any public API), this changes behavior from warning only once and
running with no current snapshot, to instead warning on each duplicate
and selecting the last one encountered (which is arbitrary based on
readdir() ordering, but actually stands a fair chance of being the
most-recently created snapshot whether by timestamp or by the
propensity of humans to name things in ascending order).

Note that the code in question is only run by libvirtd when it first
starts, reading state from disk from the previous run into memory for
this run. Since the data resides somewhere that only libvirt should be
touching (typically /var/lib/libvirt/qemu/snapshot/*), it should be
clean.  So in the common case, the code touched here is unreachable.
But if someone is actually messing with files behind libvirt's back,
they deserve the change in behavior.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      40bc98dd
    • E
      snapshot: Drop virDomainSnapshotDef.current · f1056279
      由 Eric Blake 提交于 
      The only use for the 'current' member of virDomainSnapshotDef was with
the PARSE/FORMAT_INTERNAL flag for controlling an internal-use
<active> element marking whether a particular snapshot definition was
current, and even then, only by the qemu driver on output, and by qemu
and test driver on input. But this duplicates vm->snapshot_current,
and gets in the way of potential simplifications to have qemu store a
single file for all snapshots rather than one file per snapshot.  Get
rid of the member by adding a bool* parameter during parse (ignored if
the PARSE_INTERNAL flag is not set), and by adding a new flag during
format (if FORMAT_INTERNAL is set, the value printed in <active>
depends on the new FORMAT_CURRENT).

Then update the qemu driver accordingly, which involves hoisting
assignments to vm->current_snapshot to occur prior to any point where
a snapshot XML file is written (although qemu kept
vm->current_snapshot and snapshot->def_current in sync by the end of
the function, they were not always identical in the middle of
functions, so the shuffling gets a bit interesting). Later patches
will clean up some of that confusing churn to vm->current_snapshot.

Note: even if later patches refactor qemu to no longer use
FORMAT_INTERNAL for output (by storing bulk snapshot XML instead), we
will always need PARSE_INTERNAL for input (because on upgrade, a new
libvirt still has to parse XML left from a previous libvirt).
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      f1056279
    • E
      snapshot: Minor cleanup to virDomainSnapshotAssignDef · 0baf6945
      由 Eric Blake 提交于 
      When a future patch converts virDomainSnapshotDef to be a virObject,
we need to be careful that converting VIR_FREE() to virObjectUnref()
does not result in double frees. Reorder the assignment of def into
the object to the point after object is in the hash table (as
otherwise the virHashAddEntry() error path would have a shot at
freeing def prematurely).
Suggested-by: NJohn Ferlan <ferlan@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>
      0baf6945
    • E
      snapshot: Tweaks to bulk dumpxml/import internals · 967eef2b
      由 Eric Blake 提交于 
      Change the return value of virDomainSnapshotObjListParse() to return
the number of snapshots imported, and allow a return of 0 (the
original proposal of adding a flag to virDomainSnapshotCreateXML
required returning an arbitrary non-NULL snapshot, but that idea was
abandoned; and by returning a count, we are no longer constrained to a
non-empty list).

Document which flags are supported (namely, just SECURE) in
virDomainSnapshotObjListFormat().
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      967eef2b
    • E
      vbox: Clean up some snapshot usage · 063042c7
      由 Eric Blake 提交于 
      An upcoming patch will be reworking virDomainSnapshotDef to have a
base class; minimize the churn by using a local variable to reduce the
number of dereferences required when acessing the domain definition
associated with the snapshot.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      063042c7
    • C
      qemu: add virQEMUCapsSetVAList · 05be8d8b
      由 Cole Robinson 提交于 
      And adjust virQEMUCapsSetList to use it. It will also be used in future
patches.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NCole Robinson <crobinso@redhat.com>
      05be8d8b
  3. 21 3月, 2019 1 次提交
    • E
      test: Avoid use-after-free on virDomainSnapshotDelete · 1db9d0ef
      由 Eric Blake 提交于 
      The following virsh command was triggering a use-after-free:

$ virsh -c test:///default '
  snapshot-create-as test s1
  snapshot-create-as test s2
  snapshot-delete --children-only test s1
  snapshot-current --name test'
Domain snapshot s1 created
Domain snapshot s2 created
Domain snapshot s1 children deleted

error: name in virGetDomainSnapshot must not be NULL

I got lucky on that run - although the error message is quite
unexpected.  On other runs, I was able to get a core dump, and
valgrind confirms there is a definitive problem.

The culprit? We were inconsistent about whether we set
vm->current_snapshot, snap->def->current, or both when updating how
the current snapshot was being tracked.  As a result, deletion did not
see that snapshot s2 was previously current, and failed to update
vm->current_snapshot, so that the next API using the current snapshot
failed because it referenced stale memory for the now-gone s2 (instead
of the intended s1).

The test driver code was copied from the qemu code (which DOES track
both pieces of state everywhere), but was purposefully simplified
because the test driver does not have to write persistent snapshot
state to the file system.  But when you realize that the only reason
snap->def->current needs to exist is when writing out one file per
snapshot for qemu, it's just as easy to state that the test driver
never has to mess with the field (rather than chasing down which
places forgot to set the field), and have vm->current_snapshot be the
sole source of truth in the test driver.

Ideally, I'd get rid of the 'current' member in virDomainSnapshotDef,
as well as the 'current_snapshot' member in virDomainDef, and instead
track the current member in virDomainSnapshotObjList, coupled with
writing ALL snapshot state for qemu in a single file (where I can use
<snapshots current='...'> as a wrapper, rather than
VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL to output <current>1</current> XML
on a per-snapshot file basis).  But that's a bigger change, so for now
I'm just patching things to avoid the test driver segfault.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
      1db9d0ef
  5. 20 3月, 2019 10 次提交
  7. 19 3月, 2019 6 次提交
  9. 18 3月, 2019 15 次提交