1. 27 3月, 2019 13 次提交
  2. 26 3月, 2019 27 次提交
    • M
      qemufirmwaretest: Produce better message on error · c34b3eef
      Michal Privoznik 提交于
      If qemuFirmwareFetchConfigs() returned more or fewer paths than
      expected all that we see is the following error message:
      
        Expected 5 paths, got 7
      
      While it is technically correct (the best kind of correct), we
      can do better:
      
        Unexpected path (i=0). Expected /some/path got /some/other/path
      Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
      Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
      c34b3eef
    • L
      qemu_hotplug: don't shutdown net device until the guest has released it · 34086fc5
      Laine Stump 提交于
      For [some unknown reason, possibly/probably pure chance], Net devices
      have been taken offline and their bandwidth tc rules cleared as the
      very first operation when detaching the device. This is contrary to
      every other type of device, where all hostside teardown is delayed
      until we receive the DEVICE_DELETED event back from qemu, indicating
      that the guest has finished with the device.
      
      This patch delays these two operations until receipt of
      DEVICE_DELETED, which removes an ugly wart from
      qemuDomainDetachDeviceLive(), and also seems to be a more correct
      sequence of events.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      34086fc5
    • L
      qemu_hotplug: delay sending DEVICE_REMOVED event until after *all* teardown · 78b03a77
      Laine Stump 提交于
      The VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event is sent after qemu has
      responded to a device_del command with a DEVICE_DELETED event. Before
      queuing the event, *some* of the final teardown of the device's
      trappings in libvirt is done, but not *all* of it. As a result, an
      application may receive and process the DEVICE_REMOVED event before
      libvirt has really finished with it.
      
      Usually this doesn't cause a problem, but it can - in the case of the
      bug report referenced below, vdsm is assigning a PCI device to a guest
      with managed='no', using livirt's virNodeDeviceDetachFlags() and
      virNodeDeviceReAttach() APIs. Immediately after receiving a
      DEVICE_REMOVED event from libvirt signalling that the device had been
      successfully unplugged, vdsm would cal virNodeDeviceReAttach() to
      unbind the device from vfio-pci and rebind it to the host driverm but
      because the event was received before libvirt had completely finished
      processing the removal, that device was still on the "activeDevs"
      list, and so virNodeDeviceReAttach() failed.
      
      Experimentation with additional debug logs proved that libvirt would
      always end up dispatching the DEVICE_REMOVED event before it had
      removed the device from activeDevs (with a *much* greater difference
      with managed='yes', since in that case the re-binding of the device
      occurred after queuing the device).
      
      Although the case of hostdev devices is the most extreme (since there
      is so much involved in tearing down the device), *all* device types
      suffer from the same problem - the DEVICE_REMOVED event is queued very
      early in the qemuDomainRemove*Device() function for all of them,
      resulting in a possibility of any application receiving the event
      before libvirt has really finished with the device.
      
      The solution is to save the device's alias (which is the only piece of
      info from the device object that is needed for the event) at the
      beginning of processing the device removal, and then queue the event
      as a final act before returning. Since all of the
      qemuDomainRemove*Device() functions (except
      qemuDomainRemoveChrDevice()) are now called exclusively from
      qemuDomainRemoveDevice() (which selects which of the subordinates to
      call in a switch statement based on the type of device), the shortest
      route to a solution is to doing the saving of alias, and later
      queueing of the event, in the higher level qemuDomainRemoveDevice(),
      and just completely remove the event-related code from all the
      subordinate functions.
      
      The single exception to this, as mentioned before, is
      qemuDomainRemoveChrDevice(), which is still called from somewhere
      other than qemuDomainRemoveDevice() (and has a separate arg used to
      trigger different behavior when the chr device has targetType ==
      GUESTFWD), so it must keep its original behavior intact, and must be
      treated differently by qemuDomainRemoveDevice() (similar to the way
      that qemuDomainDetachDeviceLive() treats chr and lease devices
      differently from all the others).
      
      Resolves: https://bugzilla.redhat.com/1658198Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      78b03a77
    • L
      qemu_hotplug: consolidate all common detach code in qemuDomainDetachDeviceLive · dd60bd62
      Laine Stump 提交于
      Now that all the qemuDomainDetachPrep*() functions look nearly
      identical at the end, we can put one copy of that identical code in
      qemuDomainDetachDeviceLive() at the point after the individual prep
      functions have been called, and remove the duplicated code from all
      the prep functions. The code to locate the target "detach" device
      based on the "match" device remains, as do all device-type-specific
      validations.
      
      Unfortunately there are a few things going on at once in this patch,
      which makes it a bit more difficult to follow than the others; it was
      just impossible to do the changes in stages and still have a
      buildable/testable tree at each step.
      
      The other changes of note:
      
      * The individual prep functions no longer need their driver or async
        args, so those are removed, as are the local "ret" variables, since
        in all cases the functions just directly return -1 or 0.
      
      * Some of the prep functions were checking for a valid alias and/or
        for attempts to detach a multifunction PCI device, but not all. In
        fact, both checks are valid (or at least harmless) for *all* device
        types, so they are removed from the prep functions, and done a
        single time in the common function.
      
        (any attempts to *create* an alias when there isn't one has been
        removed, since that is doomed to failure anyway; the only way the
        device wouldn't have an alias is if 1) the domain was created by
        calling virsh qemu-attach to attach an existing qemu process to
        libvirt, and 2) the qemu command that started said process used "old
        style" arguments for creating devices that didn't have any device
        ids. Even if we constructed a device id for one of these devices,
        qemu wouldn't recognize it in the device_del command anyway, so we
        may as well fail earlier with "device missing alias" rather than
        failing later with "couldn't delete device net0".)
      
      * Only one type of device has shutdown code that must not be called
        until after *all* validation of the device is done (including
        checking for multifunction PCI and valid alias, which is done in the
        toplevel common code). For this reason, the Net function has been
        split in two, with the 2nd half (qemuDomainDetachShutdownNet())
        called from the common function, right before sending the delete
        command to qemu.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      dd60bd62
    • L
      qemu_hotplug: audit *all* auditable device types in qemuDomainRemoveAuditDevice · 444c5e7c
      Laine Stump 提交于
      Although all hotpluggable devices other than lease, controller,
      watchdof, and vsock can be audited, and *are* audited when an unplug
      is successful, only disk, net, and hostdev were actually being audited
      on failure.
      
      This patch corrects that omission.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      444c5e7c
    • L
      qemu_hotplug: new function qemuDomainRemoveAuditDevice() · b914e0ec
      Laine Stump 提交于
      This function can be called with a virDomainDevicePtr and whether or
      not the removal was successful, and it will call the appropriate
      virDomainAudit*() function with the appropriate args for whatever type
      of device it's given (or do nothing, if that's appropriate). This
      permits generalizing some code that currently has a separate copy for
      each type of device.
      
      NB: Although the function initially will be called only with
      success=false, that has been made an argument so that in the future
      (when the qemuDomainRemove*Device() functions have had their common
      functionality consolidated into qemuDomainRemoveDevice()), this new
      common code can call qemuDomainRemoveAuditDevice() for all types.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      b914e0ec
    • L
      qemu_hotplug: rename Chr and Lease Detach functions · e1949c70
      Laine Stump 提交于
      qemuDomainDetachDeviceChr and qemuDomainDetachDeviceLease are more
      consistent with each other.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      e1949c70
    • L
      qemu_hotplug: standardize the names/args/calling of qemuDomainDetach*() · b6a53bf9
      Laine Stump 提交于
      Most of these functions will soon contain only some setup for
      detaching the device, not the detach code proper (since that code is
      identical for these devices). Their device specific functions are all
      being renamed to qemuDomainDetachPrep*(), where * is the
      name of that device's data member in the virDomainDeviceDef
      object.
      
      Since there will be other code in qemuDomainDetachDeviceLive() after
      the calls to qemuDomainDetachPrep*() that could still fail, we no
      longer directly set "ret" with the return code from
      qemuDomainDetachPrep*() functions, but simply return -1 on
      failure, and wait until the end of qemuDomainDetachDeviceLive() to set
      ret = 0.
      
      Along with the rename, qemuDomainDetachPrep*() functions are also
      given similar arglists, including an arg called "match" that points to
      the proto-object of the device we want to delete, and another arg
      "detach" that is used to return a pointer to the actual object that
      will be (for now *has been*) detached. To make sure these new args
      aren't confused with existing local pointers that sometimes had the
      same name (detach), the local pointer to the device is now named after
      the device type ("controller", "disk", etc). These point to the same
      place as (*detach)->data.blah, it's just easier on the eyes to have,
      e.g., "disk->dst" rather than "(*detach)->data.disk-dst".
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      b6a53bf9
    • L
      qemu_hotplug: separate Chr|Lease from other devices in DetachDevice switch · 2ec6faea
      Laine Stump 提交于
      The Chr and Lease devices have detach code that is too different from
      the other device types to handle with common functionality (which will
      soon be added at the end of qemuDomainDetachDeviceLive(). In order to
      make this difference obvious, move the cases for those two device
      types to the top of the switch statement in
      qemuDomainDetachDeviceLive(), have the cases return immediately so the
      future common code at the end of the function will be skipped, and
      also include some hopefully helpful comments to remind future
      maintainers why these two device types are treated differently.
      
      Any attempt to detach an unsupported device type should also skip the
      future common code at the end of the function, so the case for
      unsupported types is similarly changed from a simple break to a return
      -1.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      2ec6faea
    • L
      qemu_hotplug: rename dev to match in qemuDomainDetachDeviceLive · c4d6a121
      Laine Stump 提交于
      I'm about to add a second virDomainDeviceDef to this function that
      will point to the actual device in the domain object. while this is
      just a partially filled-in example of what to look for. Naming it
      match will make the code easier to follow.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      c4d6a121
    • L
      qemu_hotplug: make Detach functions called only from qemu_hotplug.c static · 637d72f9
      Laine Stump 提交于
      These are no longer called from qemu_driver.c, since the function that
      called them (qemuDomainDetachDeviceLive()) has been moved to
      qemu_hotplug.c, and they are no longer called from testqemuhotplug.c
      because it now just called qemuDomainDetachDeviceLive() instead of all
      the subordinate functions.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      637d72f9
    • L
      test: replace calls to individual detach functions with one call to main detach · d3aab990
      Laine Stump 提交于
      The individual qemuDomainDetach*Device() functions will soon be "less
      functional", since some of the code that is duplicated in 10 of the 12
      detach functions is going to be moved into the common
      qemuDomainDetachDeviceLive(), which calls them all.
      
      qemuhotplugtest.c is the only place any of these individual functions
      is called other than qemuDomainDetachDeviceLive() itself. Fortunately,
      qemuDomainDetachDeviceLive() provides exactly the functionality needed
      by the test driver (except that it supports detach of more device
      types than the test driver has tests for).
      
      This patch replaces the calls to
      qemuDomainDetach(Chr|Shmen|Watchdog|Disk)Device with a single call to
      the higher level function, allowing us to shift functionality between
      the lower level functions without breaking the tests.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      d3aab990
    • L
      qemu_hotplug: pull qemuDomainUpdateDeviceList out of qemuDomainDetachDeviceLive · b2049418
      Laine Stump 提交于
      qemuDomainDetachDeviceLive() is called from two places in
      qemu_driver.c, and qemuDomainUpdateDeviceList() is called from the
      end of qemuDomainDetachDeviceLive(), which is now in qemu_hotplug.c
      
      This patch replaces the single call to qemuDomainUpdateDeviceList()
      with two calls to it immediately after return from
      qemuDomainDetachDeviceLive(). This is only done if the return from
      that function is exactly 0, in order to exactly preserve previous
      behavior.
      
      Removing that one call from qemuDomainDetachDeviceList() will permit
      us to call it from the test driver hotplug test, replacing the
      separate calls to qemuDomainDetachDeviceDiskLive(),
      qemuDomainDetachChrDevice(), qemuDomainDetachShmemDevice() and
      qemuDomainDetachWatchdog(). We want to do this so that part of the
      common functionality of those three functions (and the rest of the
      device-specific Detach functions) can be pulled up into
      qemuDomainDetachDeviceLive() without breaking the test. (This is done
      in the next patch).
      
      NB: Almost certainly this is "not the best place" to call
      qemuDomainUpdateDeviceList() (actually, it is provably the *wrong*
      place), since it's purpose is to retrieve an "up to date" list of
      aliases for all devices from qemu, and if the guest OS hasn't yet
      processed the detach request, the now-being-removed device may still
      be on that list. It would arguably be better to instead call
      qemuDomainUpdateDevicesList() later during the response to the
      DEVICE_DELETED event for the device. But removing the call from the
      current point in the detach could have some unforeseen ill effect due
      to changed timing, so the change to move it into
      qemuDomainRemove*Device() will be done in a separate patch (in order
      to make it easily revertible in case it causes a regression).
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      b2049418
    • L
      qemu_hotplug: remove extra function in middle of DetachController call chain · e4d96324
      Laine Stump 提交于
      qemuDomainDetachDeviceControllerLive() just checks if the controller
      type is SCSI, and then either returns failure, or calls
      qemuDomainDetachControllerDevice().
      
      Instead, lets just check for type != SCSI at the top of the latter
      function, and call it directly.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      e4d96324
    • L
      qemu_hotplug: move qemuDomainDetachDeviceLive() to qemu_hotplug.c · 6a9c3fba
      Laine Stump 提交于
      This function is going to take on some of the functionality of its
      subordinate functions, which all live in qemu_hotplug.c.
      
      qemuDomainDetachDeviceControllerLive() is only called from
      qemuDomainDetachDeviceLive() (and will soon be merged into
      qemuDomainDetachControllerDevice(), which is in qemu_hotplug.c), so
      it is also moved.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      6a9c3fba
    • P
    • P
      qemu: Assume that 'set_password' and 'expire_password' are supported · ac5b6cfe
      Peter Krempa 提交于
      They were added in qemu commit 7572150c189c6553c2448334116ab717680de66d
      released in v0.14.0.
      Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
      Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
      Reviewed-by: NJán Tomko <jtomko@redhat.com>
      ac5b6cfe
    • E
      snapshot: Make virDomainMomentObjListGetNames more generic · a3d17980
      Eric Blake 提交于
      Rather than hard-coding the snapshot filter bit values into the
      generic code, add another layer of indirection: callers must map which
      of their public filter bits correspond to supported moment bits, then
      pass two separate flags (the ones translated for moment code to
      operate on, and the remaining ones for the filter callback to operate
      on).
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
      a3d17980
    • L
      qemu_hotplug: move (Attach|Detach)Lease functions with others of same type · 015e71c5
      Laine Stump 提交于
      The Attach and Detach Lease functions were together in the middle of
      the Detach functions. Put them at the end of their respective
      sections, since they behave differently from the other attach/detach
      functions (DetachLease doesn't use qemuDomainDeleteDevice(), and is
      always synchronous).
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      015e71c5
    • L
      qemu_hotplug: move (almost) all qemuDomainDetach*() functions together · 5a8ffaec
      Laine Stump 提交于
      There were two outliers at the end of the file beyond the Vcpu
      functions.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      5a8ffaec
    • L
      qemu_hotplug: move qemuDomainChangeGraphicsPasswords() · 036a4521
      Laine Stump 提交于
      It was sitting down in the middle of all the qemuDomainDetach*()
      functions. Move it up with the rest of the qemuDomain*Graphics*()
      functions.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      036a4521
    • L
      qemu_hotplug: merge qemuDomainDetachThisHostDevice into qemuDomainDetachHostDevice · 6be24148
      Laine Stump 提交于
      It's now only called from one place, and combining the two functions
      highlights the similarity with Detach functions for other device
      types.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      6be24148
    • L
      qemu_hotplug: don't call DetachThisHostDevice for hostdev network devices · 48a26681
      Laine Stump 提交于
      Back in the bad old days different device types required a different
      qemu monitor call to detach them, and so an <interface type='hostdev'>
      needed to call the function for detaching hostdevs, while other
      <interface> types could be deleted as netdevs.
      
      Times have changed, and *all* device types are detached by calling the
      common function qemuDomainDeleteDevice(vm, alias), so we don't need to
      differentiate between hostdev interfaces and the others for that
      reason.
      
      There are a few other netdev-specific functions called during
      qemuDomainDetachNetDevice() (clearing bandwidth limits, stopping the
      interface), but those turn into NOPs when type=hostdev, so they're
      safe to call for type=hostdev.
      
      The only thing that is different + not a NOP is the call to
      virDomainAudit*() when qemuDomainDeleteDevice() fails, so if we add a
      conditional for that small bit of code, we can eliminate the callout
      from qemuDomainDetachNetDevice() to qemuDomainDetachThisDevice(),
      which makes this function fit the desired pattern for merging with the
      other detach functions, and paves the way to simplifying
      qemuDomainDetachHostDevice() too.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      48a26681
    • L
      qemu_hotplug: refactor qemuDomainDetachDiskLive and qemuDomainDetachDiskDevice · ac442713
      Laine Stump 提交于
      qemuDomainDetachDiskDevice() is only called from one place. Moving the
      contents of the function to that place makes
      qemuDomainDetachDiskLive() more similar to the other Detach functions
      called by the toplevel qemuDomainDetachDevice().
      
      The goal is to make each of the device-type-specific functions do this:
      
        1) find the exact device
        2) do any device-specific validation
        3) do general validation
        4) do device-specific shutdown (only needed for net devices)
        5) do the common block of code to send device_del to qemu, then
           optionally wait for a corresponding DEVICE_DELETED event from
           qemu.
      
      with the final aim being that only items 1 & 2 will remain in each
      device-type-specific function, while 3 & 5 (which are the same for
      almost every type) will be de-duplicated and moved to the toplevel
      function that calls all of these (qemuDomainDetachDeviceLive(), which
      will also contain a callout to the one instance of (4) (netdev).
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      ac442713
    • L
      qemu_hotplug: eliminate unnecessary call to qemuDomainDetachNetDevice() · 1ed46f3a
      Laine Stump 提交于
      qemuDomainDetachHostDevice() has a check at the end that calls
      qemuDomainDetachNetDevice() in the case that the hostdev is actually a
      Net device of type='hostdev'. A long time ago when device removal was
      (supposedly but not actually) synchronous, this would cause some extra
      code to be run prior to removing the device (e.g. restoring the original MAC
      address of the device, undoing some sort of virtual port profile, etc).
      
      For quite awhile now the device removal has been asynchronous, so that
      "extra teardown" isn't handled by the detach function, but instead is
      handled by the Remove function called at a later time. The result is
      that when we call qemuDomainDetachNetDevice() from
      qemuDomainDetachHostDevice(), it ends up just calling
      qemuDomainDetachThisHostDevice() and returning, which is exactly what
      we do for all other hostdevs anyway.
      
      Based on that, remove the behavioral difference when parent.type ==
      VIR_DOMAIN_DEVICE_NET, and just call qemuDomainDetachThisHostDevice()
      for all hostdevs.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      1ed46f3a
    • L
      qemu_hotplug: eliminate multiple identical qemuDomainDetachHost*Device() functions · 287415e2
      Laine Stump 提交于
      There are separate Detach functions for PCI, USB, SCSI, Vhost, and
      Mediated hostdevs, but the functions are all 100% the same code,
      except that the PCI function checks for the guest side of the device
      being a PCI Multifunction device, while the other 4 check that the
      device's alias != NULL.
      
      The check for multifunction PCI devices should be done for *all*
      devices that are connected to the PCI bus in the guest, not just PCI
      hostdevs, and qemuIsMultiFunctionDevice() conveniently returns false
      if the queried device doesn't connect with PCI, so it is safe to make
      this check for all hostdev devices. (It also needs to be done for many
      other device types, but that will be addressed in a future patch).
      
      Likewise, since all hostdevs are detached by calling
      qemuDomainDeleteDevice(), which requires the device's alias, checking
      for a valid alias is a reasonable thing for PCI hostdevs too.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      Reviewed-by: NJán Tomko <jtomko@redhat.com>
      287415e2
    • L
      qemu_hotplug: rename a virDomainDeviceInfoPtr to avoid confusion · 1c2866a1
      Laine Stump 提交于
      Having an InfoPtr named "dev" made my brain hurt. Renaming it to
      "info" gives one less thing to confuse when looking at the code.
      Signed-off-by: NLaine Stump <laine@laine.org>
      ACKed-by: NPeter Krempa <pkrempa@redhat.com>
      1c2866a1