1. 01 7月, 2019 1 次提交
  2. 20 6月, 2019 2 次提交
  3. 19 6月, 2019 2 次提交
  4. 12 6月, 2019 5 次提交
  5. 07 6月, 2019 1 次提交
  6. 04 6月, 2019 1 次提交
  7. 27 5月, 2019 1 次提交
  8. 24 5月, 2019 3 次提交
  9. 13 5月, 2019 1 次提交
  10. 09 5月, 2019 3 次提交
    • E
      snapshot: Make virDomainSnapshotDef a virObject · 57387ff5
      Eric Blake 提交于
      This brings about a couple of benefits:
      - use of VIR_AUTOUNREF() simplifies several callers
      - Fixes a todo about virDomainMomentObjList not being polymorphic enough
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Acked-by: NPeter Krempa <pkrempa@redhat.com>
      57387ff5
    • E
      snapshot: s/current/parent/ as prep for virObject · 098043ed
      Eric Blake 提交于
      VIR_CLASS_NEW insists that descendents of virObject have 'parent' as
      the name of their inherited base class member at offset 0. While it
      would be possible to write a new class-creation macro that takes the
      actual field name 'current', and rewrite VIR_CLASS_NEW to call the new
      macro with the hard-coded name 'parent', it seems less confusing if
      all object code uses similar naming. Thus, this is a mechanical rename
      in preparation of making virDomainSnapshotDef a descendent of
      virObject.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Acked-by: NPeter Krempa <pkrempa@redhat.com>
      098043ed
    • E
      snapshot: s/parent/parent_name/ as prep for virObject · 36603bc5
      Eric Blake 提交于
      VIR_CLASS_NEW insists that descendents of virObject have 'parent' as
      the name of their inherited base class member at offset 0. While it
      would be possible to write a new class-creation macro that takes the
      actual field name, and rewrite VIR_CLASS_NEW to call the new macro
      with the hard-coded name 'parent', so that we could make
      virDomainMomentDef use a custom name for its base class, it seems less
      confusing if all object code uses similar naming. Thus, this is a
      mechanical rename in preparation of making virDomainSnapshotDef a
      descendent of virObject, when we can no longer use 'parent' for a
      different purpose than the base class.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Acked-by: NPeter Krempa <pkrempa@redhat.com>
      36603bc5
  11. 05 5月, 2019 1 次提交
  12. 01 5月, 2019 1 次提交
  13. 27 3月, 2019 1 次提交
  14. 22 3月, 2019 7 次提交
    • E
      snapshot: Rename virDomainSnapshotObjPtr · e055a816
      Eric Blake 提交于
      Now that the core of SnapshotObj is agnostic to snapshots and can be
      shared with upcoming checkpoint code, it is time to rename the struct
      and the functions specific to list operations. A later patch will
      shuffle which file holds the common code. This is a fairly mechanical
      patch.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      e055a816
    • E
      snapshot: Switch type of virDomainSnapshotObj.def · 1ab05da2
      Eric Blake 提交于
      Another step towards making the object list reusable for both
      snapshots and checkpoints: the list code only ever needs items that
      are in the common virDomainMomentDef base type. This undoes a lot of
      the churn in accessing common members added in the previous patch, and
      the bulk of the patch is mechanical. But there was one spot where I
      had to unroll a VIR_STEAL_PTR to work around changed types.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      1ab05da2
    • E
      snapshot: Factor out virDomainMomentDef class · ffc0fbeb
      Eric Blake 提交于
      Pull out the common parts of virDomainSnapshotDef that will be reused
      for virDomainCheckpointDef into a new base class.  Adjust all callers
      that use the direct fields (some of it is churn that disappears when
      the next patch refactors virDomainSnapshotObj; oh well...).
      
      Someday, I hope to switch this type to be a subclass of virObject, but
      that requires a more thorough audit of cleanup paths, and besides
      minimal incremental changes are easier to review.
      
      As for the choice of naming:
      I promised my teenage daughter Evelyn that I'd give her credit for her
      contribution to this commit. I asked her "What would be a good name
      for a base class for DomainSnapshot and DomainCheckpoint". After
      explaining what a base class was (using the classic OOB Square and
      Circle inherit from Shape), she came up with "DomainMoment", which is
      way better than my initial thought of "DomainPointInTime" or
      "DomainPIT".
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      ffc0fbeb
    • E
      snapshot: Add accessors for updating snapshot list relations · 02c4e24d
      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 finishes the job started in the previous patch, by getting
      rid of all direct access to nchildren, first_child, or sibling outside
      of the lowest level functions, making it easier to refactor later on.
      
      The lone new caller to virDomainSnapshotObjListSize() checks for a
      return != 0, because it wants to handles errors (-1, only possible if
      the hash table wasn't allocated) and existing snapshots (> 0) in the
      same manner; we can drop the check for a current snapshot on the
      grounds that there shouldn't be one if there are no snapshots.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      02c4e24d
    • 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: 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
  15. 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
  16. 16 3月, 2019 1 次提交
  17. 15 3月, 2019 1 次提交
  18. 13 3月, 2019 1 次提交
  19. 08 3月, 2019 2 次提交
    • E
      snapshot: Give virDomainSnapshotDefFormat its own flags · c5029559
      Eric Blake 提交于
      virDomainSnapshotDefFormat currently takes two sets of knobs:
      an 'unsigned int flags' argument that can currently just be
      VIR_DOMAIN_DEF_FORMAT_SECURE, and an 'int internal' argument used as
      a bool to determine whether to output an additional element.  It
      then reuses the 'flags' knob to call into virDomainDefFormatInternal(),
      which takes a different set of flags. In fact, prior to commit 0ecd6851
      (1.2.12), the 'flags' argument actually took the public
      VIR_DOMAIN_XML_SECURE, which was even more confusing.  Let's borrow
      from the style of that earlier commit, by introducing a function
      for translating from the public flags (VIR_DOMAIN_SNAPSHOT_XML_SECURE
      was just recently introduced) into a new enum specific to snapshot
      formatting, and adjust all callers to use snapshot-specific enum
      values when formatting, and where the formatter now uses a new
      variable 'domainflags' to make it obvious when we are translating
      from snapshot flags back to domain flags.  We don't even have to
      use the conversion function for drivers that don't accept the
      public VIR_DOMAIN_SNAPSHOT_XML_SECURE flag.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      c5029559
    • E
      snapshot: Rework virDomainSnapshotState enum · f43eb680
      Eric Blake 提交于
      The existing virDomainSnapshotState is a superset of virDomainState,
      adding one more state (disk-snapshot) on top of valid domain states.
      But as written, the enum cannot be used for gcc validation that all
      enum values are covered in a strongly-typed switch condition, because
      the enum does not explicitly include the values it is adding to.
      
      Copy the style used in qemu_blockjob.h of creating new enum names
      for every inherited value, and update most clients to use the new
      enum names anywhere snapshot state is referenced. The exception is
      two switch statements in qemu code, which instead gain a fixme
      comment about odd type usage (which will be cleaned up in the next
      patch). The rest of the patch is fairly mechanical (I actually did
      it by temporarily s/state/xstate/ in snapshot_conf.h to let the
      compiler find which spots in the code used the field, did the
      obvious search and replace in those functions, then undid the rename).
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      f43eb680
  20. 20 2月, 2019 2 次提交
    • E
      snapshot: Define explicit flags for snapshot xml · a27031c4
      Eric Blake 提交于
      Commit f609cb85 (0.9.5) introduced virDomainSnapshotGetXMLDesc()'s use
      of @flags as a subset of virDomainXMLFlags, documenting that 2 of the
      3 flags defined at the time would never be valid.  Later, commit
      28f8dfdc (1.0.0) introduced a new flag, VIR_DOMAIN_XML_MIGRATABLE, but
      did not adjust the snapshot documentation to declare it as invalid.
      However, since the flag is not accepted as valid by any of the
      drivers (remote is just passthrough; esx and vbox don't support flags;
      qemu, test, and vz only support VIR_DOMAIN_XML_SECURE), and it is
      unlikely that the domain state saved off during a snapshot creation
      needs to be migration-friendly (as the snapshot is not the source of
      a migration), it is easier to just define an explicit set of supported
      flags directly related to the snapshot API rather than trying to
      borrow from domain API, and risking confusion if even more domain
      flags are added later (in fact, I have an upcoming patch that plans to
      add a new flag to virDomainGetXMLDesc that makes no sense for
      snapshots).
      
      There is no API or ABI impact (since we purposefully used unsigned int
      rather than an enum type in public API, and since the new flag name
      carries the same value as the reused name).
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      a27031c4
    • E
      domain: Fix unknown flags diagnosis in virDomainGetXMLDesc · 27c8fd74
      Eric Blake 提交于
      Many drivers had a comment that they did not validate the incoming
      'flags' to virDomainGetXMLDesc() because they were relying on
      virDomainDefFormat() to do it instead. This used to be the case
      (at least since 461e0f1a and friends in 0.9.4 added unknown flag
      checking in general), but regressed in commit 0ecd6851 (1.2.12),
      when all of the drivers were changed to pass 'flags' through the
      new helper virDomainDefFormatConvertXMLFlags(). Since this helper
      silently ignores unknown flags, we need to implement flag checking
      in each driver instead.
      
      Annoyingly, this means that any new flag values added will silently
      be ignored when targeting an older libvirt, rather than our usual
      practice of loudly diagnosing an unsupported flag.  Add comments
      in domain_conf.[ch] to remind us to be extra vigilant about the
      impact when adding flags (a new flag to add data is safe if the
      older server omitting the requested data doesn't break things in
      the newer client; a new flag to suppress data rather than enhancing
      the existing VIR_DOMAIN_XML_SECURE may form a data leak or even a
      security hole).
      
      In the qemu driver, there are multiple callers all funnelling to
      qemuDomainDefFormatBufInternal(); many of them already validated
      flags (and often only a subset of the full set of possible flags),
      but for ease of maintenance, we can also check flags at the common
      helper function.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      27c8fd74
  21. 12 2月, 2019 2 次提交