1. 20 2月, 2013 5 次提交
  2. 19 2月, 2013 5 次提交
    • J
      rpc: Avoid deadlock when closing client connection · 921af429
      Jiri Denemark 提交于
      We need to drop the server lock before calling virObjectUnlock(client)
      since in case we had the last reference to the client, its dispose
      callback would be called and that could possibly try to lock the server
      and cause a deadlock. This is exactly what happens when there is only
      one QEMU domain running and it is marked to be autodestroyed when the
      connection dies. This results in qemuProcessAutoDestroy ->
      qemuProcessStop -> virNetServerRemoveShutdownInhibition call sequence,
      where the last function locks the server.
      921af429
    • J
      Avoid resetting errors in virTypedParamsFree · ee1d6d91
      Jiri Denemark 提交于
      The function does not report any errors so there should be no need too
      reset an existing error first. Moreover, virTypedParamsFree is mostly
      called in cleanup phase where it has the potential to reset any useful
      reported earlier.
      ee1d6d91
    • E
      build: force correct gcc syntax for attribute_nonnull · e086deda
      Eric Blake 提交于
      Gcc lets you do:
      
      int ATTRIBUTE_NONNULL(1) foo(void *param);
      int foo(void *param) ATTRIBUTE_NONNULL(1);
      int ATTRIBUTE_NONNULL(1) foo(void *param) { ... }
      
      but chokes on:
      
      int foo(void *param) ATTRIBUTE_NONNULL(1) { ... }
      
      However, since commit eefb881d, we have intentionally been disabling
      ATTRIBUTE_NONNULL because of lame gcc handling of the attribute (that
      is, gcc doesn't do decent warning reporting, then compiles code that
      mysteriously fails if you break the contract of the attribute, which
      is surprisingly easy to do), leaving it on only for Coverity (which
      does a much better job of improved static analysis when the attribute
      is present).
      
      But completely eliding the macro makes it too easy to write code that
      uses the fourth syntax option, if you aren't using Coverity.  So this
      patch forces us to avoid syntax errors, even when not using the
      attribute under gcc.  It also documents WHY we disable the warning
      under gcc, rather than forcing you to find the commit log.
      
      * src/internal.h (ATTRIBUTE_NONNULL): Expand to empty attribute,
      rather than nothing, when on gcc.
      e086deda
    • G
      qemu: pass "-1" as uid/gid for unprivileged qemu · 272be1a8
      Guido Günther 提交于
      so we don't try to change uid/git to 0 when probing capabilities.
      272be1a8
    • D
      Add capabilities bit for -no-kvm-pit-reinjection · 41046256
      Doug Goldstein 提交于
      The conversion to qemuCaps dropped the ability with qemu{,-kvm} 1.2 and
      newer to set the lost tick policy for the PIT. While the
      -no-kvm-pit-reinjection option is depreacated, it is still supported at
      least through 1.4, it is better to not lose the functionality.
      41046256
  3. 18 2月, 2013 1 次提交
  4. 16 2月, 2013 11 次提交
    • J
      security: Remove unnecessary checks for mgr == NULL · 676688b6
      John Ferlan 提交于
      Coverity found the DACGenLabel was checking for mgr == NULL after a
      possible dereference; however, in order to get into the function the
      virSecurityManagerGenLabel would have already dereferenced sec_managers[i]
      so the check was unnecessary. Same check is made in SELinuxGenSecurityLabel.
      676688b6
    • J
      vircommand: Remove unnecessary sa_assert · 277aaeee
      John Ferlan 提交于
      Changes from commit '3178df9a' removed the need for the sa_assert(infd).
      277aaeee
    • S
      Fix libvirt upgrade path when nwfilter is used · b7d00de2
      Stefan Berger 提交于
      Between revision 65fb9d49 and before this patch, an upgrade of libvirt while
      VMs are running and instantiating iptables filtering rules due to nwfilter
      rules, may leave stray iptables rules behind when shutting VMs down.
      Left-over iptables rules may look like this:
      
      Chain FP-vnet0 (1 references)
      target     prot opt source               destination         
      DROP       tcp  --  0.0.0.0/0            0.0.0.0/0            tcp spt:122
      ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0           
      
      [...]
      
      Chain libvirt-out (1 references)
      target     prot opt source               destination         
      FO-vnet0   all  --  0.0.0.0/0            0.0.0.0/0           [goto]  PHYSDEV match --physdev-out vnet0
      
      
      
      The reason is that the recent nwfilter code only removed filtering rules in
      the libvirt-out chain that contain the --physdev-is-bridged parameter.
      Older rules didn't match and were not removed.
      
      Note that the user-defined chain FO-vnet0 could not be removed due to the
      reference from the rule in libvirt-out.
      
      Often the work around may be done through
      
      service iptables restart
      kill -SIGHUP $(pidof libvirtd)
      
      This patch now also removes older libvirt versions' iptables rules.
      Signed-off-by: NStefan Berger <stefanb@linux.vnet.ibm.com>
      b7d00de2
    • E
      storage: don't follow backing chain symlinks too eagerly · d1333dd0
      Eric Blake 提交于
      If you have a qcow2 file /path1/to/file pointed to by symlink
      /path2/symlink, and pass qemu /path2/symlink, then qemu treats
      a relative backing file in the qcow2 metadata as being relative
      to /path2, not /path1/to.  Yes, this means that it is possible
      to create a qcow2 file where the choice of WHICH directory and
      symlink you access its contents from will then determine WHICH
      backing file (if any) you actually find; the results can be
      rather screwy, but we have to match what qemu does.
      
      Libvirt and qemu default to creating absolute backing file
      names, so most users don't hit this.  But at least VDSM uses
      symlinks and relative backing names alongside the
      --reuse-external flags to libvirt snapshot operations, with the
      result that libvirt was failing to follow the intended chain of
      backing files, and then backing files were not granted the
      necessary sVirt permissions to be opened by qemu.
      
      See https://bugzilla.redhat.com/show_bug.cgi?id=903248 for
      more gory details.  This fixes a regression introduced in
      commit 82507838.
      
      I tested this patch by creating the following chain:
      
      ls /home/eblake/Downloads/Fedora.iso # raw file for base
      cd /var/lib/libvirt/images
      qemu-img create -f qcow2 \
        -obacking_file=/home/eblake/Downloads/Fedora.iso,backing_fmt=raw one
      mkdir sub
      cd sub
      ln -s ../one onelink
      qemu-img create -f qcow2 \
        -obacking_file=../sub/onelink,backing_fmt=qcow2 two
      mv two ..
      ln -s ../two twolink
      qemu-img create -f qcow2 \
        -obacking_file=../sub/twolink,backing_fmt=qcow2 three
      mv three ..
      ln -s ../three threelink
      
      then pointing my domain at /var/lib/libvirt/images/sub/threelink.
      Prior to this patch, I got complaints about missing backing
      files; afterwards, I was able to verify that the backing chain
      (and hence DAC and SELinux relabels) of the entire chain worked.
      
      * src/util/virstoragefile.h (_virStorageFileMetadata): Add
      directory member.
      * src/util/virstoragefile.c (absolutePathFromBaseFile): Drop,
      replaced by...
      (virFindBackingFile): ...better function.
      (virStorageFileGetMetadataInternal): Add an argument.
      (virStorageFileGetMetadataFromFD, virStorageFileChainLookup)
      (virStorageFileGetMetadata): Update callers.
      d1333dd0
    • E
      storage: refactor metadata lookup · 2485f921
      Eric Blake 提交于
      Prior to this patch, we had the callchains:
      external users
        \-> virStorageFileGetMetadataFromFD
            \-> virStorageFileGetMetadataFromBuf
      virStorageFileGetMetadataRecurse
        \-> virStorageFileGetMetadataFromFD
            \-> virStorageFileGetMetadataFromBuf
      
      However, a future patch wants to add an additional parameter to
      the bottom of the chain, for use by virStorageFileGetMetadataRecurse,
      without affecting existing external callers.  Since there is only a
      single caller of the internal function, we can repurpose it to fit
      our needs, with this patch giving us:
      
      external users
        \-> virStorageFileGetMetadataFromFD
            \-> virStorageFileGetMetadataInternal
      virStorageFileGetMetadataRecurse /
        \-> virStorageFileGetMetadataInternal
      
      * src/util/virstoragefile.c (virStorageFileGetMetadataFromFD):
      Move most of the guts...
      (virStorageFileGetMetadataFromBuf): ...here, and rename...
      (virStorageFileGetMetadataInternal): ...to this.
      (virStorageFileGetMetadataRecurse): Use internal helper.
      2485f921
    • E
      storage: prepare for refactoring · b7df4f92
      Eric Blake 提交于
      virStorageFileGetMetadataFromFD is the only caller of
      virStorageFileGetMetadataFromBuf; and it doesn't care about the
      difference between a return of 0 (total success) or 1
      (metadata was inconsistent, but pointer was populated as best
      as possible); only about a return of -1 (could not read metadata
      or out of memory).  Changing the return type, and normalizing
      the variable names used, will make merging the functions easier
      in the next commit.
      
      * src/util/virstoragefile.c (virStorageFileGetMetadataFromBuf):
      Change return value, and rename some variables.
      (virStorageFileGetMetadataFromFD): Rename some variables.
      b7df4f92
    • E
      storage: rearrange functions · 5e4946d4
      Eric Blake 提交于
      No semantic change; done so the next patch doesn't need a forward
      declaration of a static function.
      
      * src/util/virstoragefile.c (virStorageFileProbeFormatFromBuf):
      Hoist earlier.
      5e4946d4
    • E
      build: more mingw fixes · c51c3e45
      Eric Blake 提交于
      More mingw build failures:
      
        CCLD     libvirt-lxc.la
      /usr/lib64/gcc/i686-w64-mingw32/4.7.2/../../../../i686-w64-mingw32/bin/ld: cannot find libvirt_lxc.def: No such file or directory
      
        CC       virportallocatortest-virportallocatortest.o
      ../../tests/virportallocatortest.c: In function 'main':
      ../../tests/virportallocatortest.c:195:1: error: implicit declaration of function 'setenv' [-Werror=implicit-function-declaration]
      
      * src/Makefile.am (GENERATED_SYM_FILES): Also generate
      libvirt_lxc.def.
      * bootstrap.conf (gnulib_modules): Import setenv.
      c51c3e45
    • E
      build: fix mingw build · 660db5bf
      Eric Blake 提交于
      Commits 20253560 and ba72cb12 introduced typos.
      
      * src/util/virpci.c (virPCIIsVirtualFunction) [!__linux__]: Fix
      function name.
      * src/util/virutil.c (virGetDeviceID): Fix attribute spelling.
      660db5bf
    • E
      build: fix vircommand build on mingw · ec2cc0f8
      Eric Blake 提交于
        CC       libvirt_util_la-vircommand.lo
      ../../src/util/vircommand.c:2358:1: error: 'virCommandHandshakeChild' defined but not used [-Werror=unused-function]
      
      The function is only implemented inside #ifndef WIN32.
      
      * src/util/vircommand.c (virCommandHandshakeChild): Hoist earlier,
      so that win32 build doesn't hit an unused forward declaration.
      ec2cc0f8
    • E
      regex: gnulib guarantees that we have regex support · ec8a2d03
      Eric Blake 提交于
      No need to use HAVE_REGEX_H - our use of gnulib guarantees that
      the header exists and works, regardless of platform.  Similarly,
      we can unconditionally assume a compiling <sys/wait.h> (although
      the mingw version of this header is not full-featured).
      
      * src/storage/storage_backend.c: Drop useless conditional.
      * tests/testutils.c: Likewise.
      ec8a2d03
  5. 14 2月, 2013 15 次提交
    • J
      2e5d7798
    • L
      util: maintain caps when running command with uid != 0 · 7a2e845a
      Laine Stump 提交于
      virCommand was previously calling virSetUIDGID() to change the uid and
      gid of the child process, then separately calling
      virSetCapabilities(). This did not work if the desired uid was != 0,
      since a setuid to anything other than 0 normally clears all
      capabilities bits.
      
      The solution is to use the new virSetUIDGIDWithCaps(), sending it the
      uid, gid, and capabilities bits. This will get the new process setup
      properly.
      
      Since the static functions virSetCapabilities() and
      virClearCapabilities are no longer called, they have been removed.
      
      NOTE: When combined with "filecap $path-to-qemu sys_rawio", this patch
      will make CAP_SYS_RAWIO (which is required for passthrough of generic
      scsi commands to a guest - see commits e8daeeb1, 177db087, 397e6a70, and
      74e03496) be retained by qemu when necessary. Apparently that
      capability has been broken for non-root qemu ever since it was
      originally added.
      7a2e845a
    • L
      util: virSetUIDGIDWithCaps - change uid while keeping caps · e11451f4
      Laine Stump 提交于
      Normally when a process' uid is changed to non-0, all the capabilities
      bits are cleared, even those explicitly set with calls to
      capng_update()/capng_apply() made immediately before setuid. And
      *after* the process' uid has been changed, it no longer has the
      necessary privileges to add capabilities back to the process.
      
      In order to set a non-0 uid while still maintaining any capabilities
      bits, it is necessary to either call capng_change_id() (which
      unfortunately doesn't currently call initgroups to setup auxiliary
      group membership), or to perform the small amount of calisthenics
      contained in the new utility function virSetUIDGIDWithCaps().
      
      Another very important difference between the capabilities
      setting/clearing in virSetUIDGIDWithCaps() and virCommand's
      virSetCapabilities() (which it will replace in the next patch) is that
      the new function properly clears the capabilities bounding set, so it
      will not be possible for a child process to set any new
      capabilities.
      
      A short description of what is done by virSetUIDGIDWithCaps():
      
      1) clear all capabilities then set all those desired by the caller (in
      capBits) plus CAP_SETGID, CAP_SETUID, and CAP_SETPCAP (which is needed
      to change the capabilities bounding set).
      
      2) call prctl(), telling it that we want to maintain current
      capabilities across an upcoming setuid().
      
      3) switch to the new uid/gid
      
      4) again call prctl(), telling it we will no longer want capabilities
      maintained if this process does another setuid().
      
      5) clear the capabilities that we added to allow us to
      setuid/setgid/change the bounding set (unless they were also requested
      by the caller via the virCommand API).
      
      Because the modification/maintaining of capabilities is intermingled
      with setting the uid, this is necessarily done in a single function,
      rather than having two independent functions.
      
      Note that, due to the way that effective capabilities are computed (at
      time of execve) for a process that has uid != 0, the *file*
      capabilities of the binary being executed must also have the desired
      capabilities bit(s) set (see "man 7 capabilities"). This can be done
      with the "filecap" command. (e.g. "filecap /usr/bin/qemu-kvm sys_rawio").
      e11451f4
    • L
      util: drop capabilities immediately after changing uid/gid of child · c0e3e685
      Laine Stump 提交于
      This is an interim measure to make sure everything still works in this
      order. The next step will be to perform capabilities drop and
      setuid/gid as a single operation (which is the only way to keep any
      capabilities when switching to a non-root uid).
      c0e3e685
    • L
      qemu: let virCommand set child process security labels/uid/gid · 0345c728
      Laine Stump 提交于
      The qemu driver had been calling virSecurityManagerSetProcessLabel()
      from a "pre-exec hook" function that is run after the child is forked,
      but before exec'ing qemu. This is problematic because the uid and gid
      of the child are set by the security driver, but capabilities are
      dropped by virCommand - such separation doesn't work; the two
      operations must be done together or the capabilities do not transfer
      properly to the child process.
      
      This patch switches to using virSecurityManagerSetChildProcessLabel(),
      which is called prior to virCommandRun() (rather than being called
      *during* virCommandrun() by the hook function), and doesn't set the
      UID/GID/security label directly, but instead merely informs virCommand
      what it should set them all to when the time is appropriate.
      
      This lets virCommand choose to do the uid/gid and caps dropping all at
      the same time if it wants (it does *want* to, but isn't doing so yet;
      that's for an upcoming patch).
      0345c728
    • L
      security: add new virSecurityManagerSetChildProcessLabel API · 7bf1aa0b
      Laine Stump 提交于
      The existing virSecurityManagerSetProcessLabel() API is designed so
      that it must be called after forking the child process, but before
      exec'ing the child. Due to the way the virCommand API works, that
      means it needs to be put in a "hook" function that virCommand is told
      to call out to at that time.
      
      Setting the child process label is a basic enough need when executing
      any process that virCommand should have a method of doing that. But
      virCommand must be told what label to set, and only the security
      driver knows the answer to that question.
      
      The new virSecurityManagerSet*Child*ProcessLabel() API is the way to
      transfer the knowledge about what label to set from the security
      driver to the virCommand object. It is given a virCommandPtr, and each
      security driver calls the appropriate virCommand* API to tell
      virCommand what to do between fork and exec.
      
      1) in the case of the DAC security driver, it calls
      virCommandSetUID/GID() to set a uid and gid that must be set for the
      child process.
      
      2) for the SELinux security driver, it calls
      virCommandSetSELinuxLabel() to save a copy of the char* that will be
      sent to setexeccon_raw() *after forking the child process*.
      
      3) for the AppArmor security drivers, it calls
      virCommandSetAppArmorProfile() to save a copy of the char* that will
      be sent to aa_change_profile() *after forking the child process*.
      
      With this new API in place, we will be able to remove
      virSecurityManagerSetProcessLabel() from any virCommand pre-exec
      hooks.
      
      (Unfortunately, the LXC driver uses clone() rather than virCommand, so
      it can't take advantage of this new security driver API, meaning that
      we need to keep around the older virSecurityManagerSetProcessLabel(),
      at least for now.)
      7bf1aa0b
    • L
      util: add security label setting to virCommand · 6c3f3d0d
      Laine Stump 提交于
      virCommand gets two new APIs: virCommandSetSELinuxLabel() and
      virCommandSetAppArmorProfile(), which both save a copy of a
      null-terminated string in the virCommand. During virCommandRun, if the
      string is non-NULL and we've been compiled with AppArmor and/or
      SELinux security driver support, the appropriate security library
      function is called for the child process, using the string that was
      previously set. In the case of SELinux, setexeccon_raw() is called,
      and for AppArmor, aa_change_profile() is called.
      
      This functionality has been added so that users of virCommand can use
      the upcoming virSecurityManagerSetChildProcessLabel() prior to running
      a child process, rather than needing to setup a hook function to be
      called (and in turn call virSecurityManagerSetProcessLabel()) *during*
      the setup of the child process.
      6c3f3d0d
    • L
      build: define SECDRIVER_LIBS in Makefile.am · 4a56e80f
      Laine Stump 提交于
      This makes it simpler to include the necessary system security driver
      libraries for a particular system. For this patch, several existing
      conditional sections from the Makfile were replaced; I'll later be
      adding SECDRIVER_LIBS to libvirt_util_la_LIBADD, because vircommand.c
      will be calling a function from $securitylib.
      4a56e80f
    • L
    • L
      qemu: replace exec hook with virCommandSetUID/GID in qemuCaps* · 6a8ecc37
      Laine Stump 提交于
      Setting the uid/gid of the child process was the only thing done by
      the hook function in this case, and that can now be done more simply
      with virCommandSetUID/GID.
      6a8ecc37
    • L
      util: make virSetUIDGID a NOP only when uid or gid is -1 · f506a4c1
      Laine Stump 提交于
      Rather than treating uid:gid of 0:0 as a NOP, we blindly pass that
      through to the lower layers. However, we *do* check for a requested
      value of "-1" to mean "don't change this setting". setregid() and
      setreuid() already interpret -1 as a NOP, so this is just an
      optimization, but we are also calling getpwuid_r and initgroups, and
      it's unclear what the former would do with a uid of -1.
      f506a4c1
    • L
      util: add virCommandSetUID and virCommandSetGID · 417182b0
      Laine Stump 提交于
      If a uid and/or gid is specified for a command, it will be set just
      after the user-supplied post-fork "hook" function is called.
      
      The intent is that this can replace user hook functions that set
      uid/gid. This moves the setting of uid/gid and dropping of
      capabilities closer to each other, which is important since the two
      should really be done at the same time (libcapng provides a single
      function that does both, which we will be unable to use, but want to
      mimic as closely as possible).
      417182b0
    • L
    • L
      util: eliminate extra args from virExec · 5f2ce539
      Laine Stump 提交于
      All args except "cmd" in the call to virExec are now redundant, since
      they can all be found in cmd, so remove the args and reference the
      data directly in cmd. One exception to this is that "infd" was being
      modified within virExec, and modifying the original in cmd caused make
      check failures, so cmd->infd is copied to a local, and the local is
      used during virExec().
      5f2ce539
    • L
      util: eliminate generic hook from virExecWithHook · b6decc57
      Laine Stump 提交于
      virExecWithHook is only called from one place, so it always has the
      same "hook" function (virHookCommand), and the data sent to that
      function is always a virCommandPtr, so eliminate the function and
      generic data from the arglist, and replace it with "virCommandPtr
      cmd". The call to (hook)(data) is replaced with
      "virHookCommand(cmd)". Finally, virExecWithHook is renamed to virExec.
      
      Indentation has been updated only for code that will remain after the
      next patch, which will remove all other args to virExec (since they
      are now redundant, as they're all members of virCommandPtr).
      b6decc57
  6. 13 2月, 2013 3 次提交
    • D
      Remove qemuDriverLock from almost everywhere · a9e97e0c
      Daniel P. Berrange 提交于
      With the majority of fields in the virQEMUDriverPtr struct
      now immutable or self-locking, there is no need for practically
      any methods to be using the QEMU driver lock. Only a handful
      of helper APIs in qemu_conf.c now need it
      a9e97e0c
    • M
      virCommand: Don't misuse the eventloop for async IO · 3178df9a
      Michal Privoznik 提交于
      Currently, if a command wants to do asynchronous IO, a callback
      is registered in the libvirtd eventloop to handle writes and
      reads. However, there's a race in virCommandWait. The eventloop
      may already be executing the callback, while virCommandWait is
      mangling internal state of virCommand. To deal with it, we need
      to either introduce locking or spawn a separate thread where we
      poll() on stdio from child. The former, however, requires to
      unlock all mutexes held, as the event loop may execute other
      callbacks which tries to lock one of the mutexes, deadlock and
      thus never wake us up. So it's safer to spawn a separate thread.
      3178df9a
    • E
      xen: clean up the mess with cpumap · 069b5c5a
      Eric Blake 提交于
      Commit 8b55992f added some Coverity comments to silence what was
      a real bug in the code.  Since then, we've had a miserable run
      of trying to fix the underlying problem (commits c059cdea and
      ba5193c8), and still have a problem on 32-bit machines.
      
      This fixes the problem for once and for all, by realizing that
      on older xen, cpumap_t is identical to uint64_t, and using the
      new virendian.h to do the transformation from the API (documented
      to be little-endian) to the host structure.
      
      * src/xen/xen_hypervisor.c (virXen_setvcpumap): Do the conversion
      correctly.  Finally.
      069b5c5a