1. 17 12月, 2019 1 次提交
  2. 13 12月, 2019 2 次提交
    • D
      qemu: honour parseOpaque instead of refetching caps · 8d157c13
      Daniel P. Berrangé 提交于
      The use of the parseOpaque parameter was mistakenly removed in
      
        commit 4a4132b4
        Author: Daniel P. Berrangé <berrange@redhat.com>
        Date:   Tue Dec 3 10:49:49 2019 +0000
      
          conf: don't use passed in caps in post parse method
      
      causing the method to re-fetch qemuCaps that were already just
      fetched and put into parseOpaque.
      
      This is inefficient when parsing incoming XML, but for live
      XML this is more serious as it means we use the capabilities
      for the current QEMU binary on disk, rather than the running
      QEMU.
      
      That commit, however, did have a useful side effect of fixing
      a crasher bug in the qemu post parse callback introduced by
      
        commit 5e939cea
        Author: Jiri Denemark <jdenemar@redhat.com>
        Date:   Thu Sep 26 18:42:02 2019 +0200
      
          qemu: Store default CPU in domain XML
      
      The qemuDomainDefSetDefaultCPU() method in that patch did not
      allow for the possibility that qemuCaps would be NULL and thus
      resulted in a SEGV.
      
      This shows a risk in letting each check in the post parse
      callback look for qemuCaps == NULL. The safer option is to
      check once upfront and immediately stop (postpone) further
      validation.
      Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
      Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>
      8d157c13
    • D
      qemu: check os type / virt type / arch in validate callback · 87a8b81d
      Daniel P. Berrangé 提交于
      Don't check os type / virt type / arch in the post-parse callback
      because we can't assume qemuCaps is non-NULL at this point. It
      also conceptually belongs to the validation callback.
      Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
      Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>
      87a8b81d
  3. 12 12月, 2019 2 次提交
    • P
      qemu: fix concurrency crash bug in snapshot revert · d75f865f
      Pavel Mores 提交于
      This commit aims to fix
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1610207
      
      The cause was apparently incorrect handling of jobs in snapshot
      revert code which allowed a thread executing snapshot delete to
      begin job while snapshot revert was still running on another
      thread. The snapshot delete thread then waited on a condition
      variable in qemuMonitorSend() while the revert thread finished,
      changing (and effectively corrupting) the qemuMonitor structure
      under the delete thread which led to its crash.
      
      The incorrect handling of jobs in revert code was due to the fact
      that although qemuDomainRevertToSnapshot() correctly begins a job
      at the start, the job was implicitly ended when qemuProcessStop()
      was called because the job lives in the QEMU driver's private
      data (qemuDomainObjPrivate) that was purged during
      qemuProcessStop().
      
      This fix prevents qemuProcessStop() from clearing jobs as the
      idea of qemuProcessStop() clearing jobs seems wrong in the first
      place. It was (inadvertently) introduced in commit
      888aa4b6, which is effectively reverted by
      the second hunk of this commit. To preserve the desired effects
      of the faulty commit, the first hunk is included as suggested by
      Michal.
      Signed-off-by: NPavel Mores <pmores@redhat.com>
      Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
      d75f865f
    • D
      qemu: keep capabilities when running QEMU as root · 19023562
      Daniel P. Berrangé 提交于
      When QEMU uid/gid is set to non-root this is pointless as if we just
      used a regular setuid/setgid call, the process will have all its
      capabilities cleared anyway by the kernel.
      
      When QEMU uid/gid is set to root, this is almost (always?) never
      what people actually want. People make QEMU run as root in order
      to access some privileged resource that libvirt doesn't support
      yet and this often requires capabilities. As a result they have
      to go find the qemu.conf param to turn this off. This is not
      viable for libguestfs - they want to control everything via the
      XML security label to request running as root regardless of the
      qemu.conf settings for user/group.
      
      Clearing capabilities was implemented originally because there
      was a proposal in Fedora to change permissions such that root,
      with no capabilities would not be able to compromise the system.
      ie a locked down root account. This never went anywhere though,
      and as a result clearing capabilities when running as root does
      not really get us any security benefit AFAICT. The root user
      can easily do something like create a cronjob, which will then
      faithfully be run with full capabilities, trivially bypassing
      the restriction we place.
      
      IOW, our clearing of capabilities is both useless from a security
      POV, and breaks valid use cases when people need to run as root.
      
      This removes the clear_emulator_capabilities configuration
      option from qemu.conf, and always runs QEMU with capabilities
      when root.  The behaviour when non-root is unchanged.
      Reviewed-by: NCole Robinson <crobinso@redhat.com>
      Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>
      19023562
  4. 11 12月, 2019 1 次提交
  5. 10 12月, 2019 8 次提交
  6. 09 12月, 2019 21 次提交
  7. 06 12月, 2019 1 次提交
  8. 03 12月, 2019 1 次提交
  9. 27 11月, 2019 1 次提交
  10. 26 11月, 2019 2 次提交
    • L
      conf: add hypervisor agnostic, domain start-time, validation function for NetDef · b03d9e95
      Laine Stump 提交于
      <interface> devices (virDomainNetDef) are a bit different from other
      types of devices in that their actual type may come from a network (in
      the form of a port connection), and that doesn't happen until the
      domain is started. This means that any validation of an <interface> at
      parse time needs to be a bit liberal in what it accepts - when
      type='network', you could think that something is/isn't allowed, but
      once the domain is started and a port is created by the configured
      network, the opposite might be true.
      
      To solve this problem hypervisor drivers need to do an extra
      validation step when the domain is being started. I recently (commit
      3cff23f7, libvirt 5.7.0) added a function to peform such validation
      for all interfaces to the QEMU driver -
      qemuDomainValidateActualNetDef() - but while that function is a good
      single point to call for the multiple places that need to "start" an
      interface (domain startup, device hotplug, device update), it can't be
      called by the other hypervisor drivers, since 1) it's in the QEMU
      driver, and 2) it contains some checks specific to QEMU. For
      validation that applies to network devices on *all* hypervisors, we
      need yet another interface validation function that can be called by
      any hypervisor driver (not just QEMU) right after its network port has
      been created during domain startup or hotplug. This patch adds that
      function - virDomainActualNetDefValidate(), in the conf directory,
      and calls it in appropriate places in the QEMU, lxc, and libxl
      drivers.
      
      This new function is the place to put all network device validation
      that 1) is hypervisor agnostic, and 2) can't be done until we know the
      "actual type" of an interface.
      
      There is no framework for validation at domain startup as there is for
      post-parse validation, but I don't want to create a whole elaborate
      system that will only be used by one type of device. For that reason,
      I just made a single function that should be called directly from the
      hypervisors, when they are initializing interfaces to start a domain,
      right after conditionally allocating the network port (and regardless
      of whether or not that was actually needed). In the case of the QEMU
      driver, qemuDomainValidateActualNetDef() is already called in all the
      appropriate places, so we can just call the new function from
      there. In the case of the other hypervisors, we search for
      virDomainNetAllocateActualDevice() (which is the hypervisor-agnostic
      function that calls virNetworkPortCreateXML()), and add the call to our
      new function right after that.
      
      The new function itself could be plunked down into many places in the
      code, but we already have 3 validation functions for network devices
      in 2 different places (not counting any basic validation done in
      virDomainNetDefParseXML() itself):
      
      1) post-parse hypervisor-agnostic
         (virDomainNetDefValidate() - domain_conf.c:6145)
      2) post-parse hypervisor-specific
         (qemuDomainDeviceDefValidateNetwork() - qemu_domain.c:5498)
      3) domain-start hypervisor-specific
         (qemuDomainValidateActualNetDef() - qemu_domain.c:5390)
      
      I placed (3) right next to (2) when I added it, specifically to avoid
      spreading validation all over the code. For the same reason, I decided
      to put this new function right next to (1) - this way if someone needs
      to add validation specific to qemu, they go to one location, and if
      they need to add validation applying to everyone, they go to the
      other. It looks a bit strange to have a public function in between a
      bunch of statics, but I think it's better than the alternative of
      further fragmentation. (I'm open to other ideas though, of course.)
      Signed-off-by: NLaine Stump <laine@redhat.com>
      Reviewed-by: NCole Robinson <crobinso@redhat.com>
      b03d9e95
    • L
      conf: return a const from virDomainNetGetActualVirtPortProfile · fdcd273b
      Laine Stump 提交于
      This also isn't required (due to the vportprofile being stored in the
      NetDef as a pointer rather than being directly contained), but it
      seemed dishonest to not mark it as const (and thus permit users to
      modify its contents)
      Signed-off-by: NLaine Stump <laine@redhat.com>
      Reviewed-by: NCole Robinson <crobinso@redhat.com>
      fdcd273b