1. 26 3月, 2019 2 次提交
    • L
      qemu_hotplug: remove another erroneous qemuDomainDetachExtensionDevice() call · e18e9b72
      Laine Stump 提交于
      qemuDomainRemoveRNGDevice() calls qemuDomainDetachExtensionDevice().
      According to commit 1d1e264f that added this code, it should not be
      necessary to explicitly remove the zPCI extension device for a PCI
      device during unplug, because "QEMU implements an unplug callback
      which will unplug both PCI and zPCI device in a cascaded way". In
      fact, no other devices call qemuDomainDetachExtensionDevice() during
      their qemuDomainRemove*Device() function, so it should be removed from
      qemuDomainRemoveRNGDevice as well.
      Signed-off-by: NLaine Stump <laine@laine.org>
      Reviewed-by: NJán Tomko <jtomko@redhat.com>
      Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
      e18e9b72
    • L
      qemu_hotplug: remove erroneous call to qemuDomainDetachExtensionDevice() · 14329169
      Laine Stump 提交于
      qemuDomainDetachControllerDevice() calls
      qemuDomainDetachExtensionDevice() when the controller type is
      PCI. This is incorrect in multiple ways:
      
      * Any code that tears down a device should be in the
        qemuDomainRemove*Device() function (which is called after libvirt
        gets a DEVICE_DELETED event from qemu indicating that the guest is
        finished with the device on its end. The qemuDomainDetach*Device()
        functions should only contain code that ensures the requested
        operation is valid, and sends the command to qemu to initiate the
        unplug.
      
      * qemuDomainDetachExtensionDevice() is a function that applies to
        devices that plug into a PCI slot, *not* necessarily PCI controllers
        (which is what's being checked in the offending code). The proper
        way to check for this would be to see if the DeviceInfo for the
        controller device had a PCI address, not to check if the controller
        is a PCI controller (the code being removed was doing the latter).
      
      * According to commit 1d1e264f that added this code (and other
        support for hotplugging zPCI devices on s390), it's not necessary to
        explicitly detach the zPCI device when unplugging a PCI device. To
        quote:
      
             There's no need to implement hot unplug for zPCI as QEMU
             implements an unplug callback which will unplug both PCI and
             zPCI device in a cascaded way.
      
        and the evidence bears this out - all the other uses of
        qemuDomainDetachExtensionDevice() (except one, which I believe is
        also in error, and is being removed in a separate patch) are only to
        remove the zPCI extension device in cases where it was successfully
        added, but there was some other failure later in the hotplug process
        (so there was no regular PCI device to remove and trigger removal of
        the zPCI extension device).
      
      * PCI controllers are not hot pluggable, so this is dead code
        anyway. (The only controllers that can currently be
        hotplugged/unplugged are SCSI controllers).
      Signed-off-by: NLaine Stump <laine@laine.org>
      Reviewed-by: NJán Tomko <jtomko@redhat.com>
      Reviewed-by: NBoris Fiuczynski <fiuczy@linux.ibm.com>
      14329169
  2. 22 3月, 2019 10 次提交
    • 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: Access snapshot def directly when needed · 55c2ab3e
      Eric Blake 提交于
      An upcoming patch will rework virDomainSnapshotObjList to be generic
      for both snapshots and checkpoints; reduce the churn by adding a new
      accessor virDomainSnapshotObjGetDef() which returns the
      snapshot-specific definition even when the list is rewritten to
      operate only on a base class, then using it at sites that that are
      specific to snapshots.  Use VIR_STEAL_PTR when appropriate in the
      affected lines.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      55c2ab3e
    • 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: 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
    • 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. 20 3月, 2019 2 次提交
  4. 18 3月, 2019 4 次提交
  5. 16 3月, 2019 1 次提交
  6. 15 3月, 2019 10 次提交
  7. 14 3月, 2019 2 次提交
  8. 13 3月, 2019 3 次提交
  9. 12 3月, 2019 6 次提交