Our fixes for CVE-2013-4400 were so effective at "fixing" bugs
in virt-login-shell that we ended up fixing it into a useless
do-nothing program.
Commit 3e2f27e1 picked the name LIBVIRT_SETUID_RPC_CLIENT for
the witness macro when we are doing secure compilation. But
commit 9cd6a57d checked whether the name IN_VIRT_LOGIN_SHELL,
from an earlier version of the patch series, was defined; with
the net result that virt-login-shell invariably detected that
it was setuid and failed virInitialize.
Commit b7fcc799 closed all fds larger than stderr, but in the
wrong place. Looking at the larger context, we mistakenly did
the close in between obtaining the set of namespace fds, then
actually using those fds to switch namespace, which means that
virt-login-shell will ALWAYS fail.
This is the minimal patch to fix the regressions, although
further patches are also worth having to clean up poor
semantics of the resulting program (for example, it is rude to
not pass on the exit status of the wrapped program back to the
invoking shell).
* tools/virt-login-shell.c (main): Don't close fds until after
namespace swap.
* src/libvirt.c (virGlobalInit): Use correct macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1049529
The 'detach-disk' command in virsh used the active XML definition of a
domain even when attempting to remove a disk from the config only. If
the disk was only in the inactive definition the operation failed. Fix
this by using the inactive XML in case that only the config is affected.
https://bugzilla.redhat.com/show_bug.cgi?id=1049529
The legacy virDomainAttachDevice and virDomainDetachDevice operate only
on active domains. When a user specified --current flag with an inactive
domain the old API was used and reported an error. Fix it by calling the
new API if --current is specified explicitly.
https://bugzilla.redhat.com/show_bug.cgi?id=1044806
Currently, sending the ANSI_A keycode from os_x codepage doesn't work as
it has a special value of 0x0. Our internal code handles that no
different to other not defined keycodes. Hence, in order to allow it we
must change all the undefined keycodes from 0 to -1 and adapt some code
too.
# virsh send-key guestname --codeset os_x ANSI_A
error: invalid keycode: 'ANSI_A'
# virsh send-key guestname --codeset os_x ANSI_B
# virsh send-key guestname --codeset os_x ANSI_C
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
I noticed a few odd things in 'virt-login-shell --help' output.
* tools/virt-login-shell.c (usage): At most one option accepted,
drop trailing colon.
Signed-off-by: Eric Blake <eblake@redhat.com>
Recent addition of the gluster pool type omitted fixing the virsh and
virConnectListAllStoragePool filters. A typecast of the converting
function in virsh showed that also the sheepdog pool was omitted in the
command parser.
This patch adds gluster pool filtering support and fixes virsh to
properly convert all supported storage pool types. The added typecast
should avoid doing such mistakes in the future.
https://bugzilla.redhat.com/show_bug.cgi?id=1044445
When undefining a VM with storage the man page doesn't explicitly
mention that the volumes need to be a part of the storage pool otherwise
it won't work.
Adding output to 'virsh --version=long' makes it easier to
tell if a distro built with particular libraries (it doesn't
tell you what a remote libvirtd is built with, but is still
better than nothing). But we forgot to mention gluster.
* tools/virsh.c (vshShowVersion): Add gluster witness.
Signed-off-by: Eric Blake <eblake@redhat.com>
Though trying to destroy a physical HBA doesn't make sense at all,
it's still a bit misleading with saying "only works for HBA".
Signed-off-by: Osier Yang <jyang@redhat.com>
Based on a suggestion from Mauricio Tavares.
* tools/virsh-domain.c (cmdDetachInterface, vshFindDisk): Improve
wording.
Signed-off-by: Eric Blake <eblake@redhat.com>
In the 'directory' and 'netfs' storage pools, a user can see
both 'file' and 'dir' storage volume types, to know when they
can descend into a subdirectory. But in a network-based storage
pool, such as the upcoming 'gluster' pool, we use 'network'
instead of 'file', and did not have any counterpart for a
directory until this patch. Adding a new volume type
'network-dir' is better than reusing 'dir', because it makes
it clear that the only way to access 'network' volumes within
that container is through the network mounting (leaving 'dir'
for something accessible in the local file system).
* include/libvirt/libvirt.h.in (virStorageVolType): Expand enum.
* docs/formatstorage.html.in: Document it.
* docs/schemasa/storagevol.rng (vol): Allow new value.
* src/conf/storage_conf.c (virStorageVol): Use new value.
* src/qemu/qemu_command.c (qemuBuildVolumeString): Fix client.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Likewise.
* tools/virsh-volume.c (vshVolumeTypeToString): Likewise.
* src/storage/storage_backend_fs.c
(virStorageBackendFileSystemVolDelete): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Most of our code base uses space after comma but not before;
fix the remaining uses before adding a syntax check.
* tests/sysinfotest.c: Consistently use commas.
* tests/viratomictest.c: Likewise.
* tests/vircgroupmock.c: Likewise.
* tools/virsh-domain.c: Likewise.
* tools/virsh-volume.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
All *-info virsh commands output a list of colon-seperated key-val pairs.
But virsh net-info command misses this colon for key "Name" and "UUID".
Signed-off-by: Hao Liu <hliu@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch shuts up the following warning of clang
on Mac OS X:
virsh.c:2761:22: error: assigning to 'char *' from 'const char [6]' discards qualifiers
[-Werror,-Wincompatible-pointer-types-discards-qualifiers]
rl_readline_name = "virsh";
^ ~~~~~~~
The warning happens because rl_readline_name on Mac OS X comes
from an old readline header that still uses 'char *', while it
is 'const char *' in readline 4.2 (April 2001) and newer.
Tested on Mac OS X 10.8.5 (clang-500.2.75) and Fedora 19 (gcc 4.8.1).
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Before:
$ virsh iface-list
Name State MAC Address
--------------------------------------------
br0 active f0🇩🇪f1:dc:b8:b0
virbr2 active 52:54:00:61:78:0c
After:
$ virsh iface-list
Name State MAC Address
---------------------------------------------------
br0 active f0🇩🇪f1:dc:b8:b0
virbr2 active 52:54:00:61:78:0c
Change the alignment to match the domain listing function.
Before:
$ virsh pool-list
Name State Autostart
-----------------------------------------
boot-scratch active no
default active no
glusterpool active no
$ virsh pool-list --details
Name State Autostart Persistent Capacity Allocation Available
-------------------------------------------------------------------------------
boot-scratch running no yes 117.99 GiB 101.40 GiB 16.60 GiB
default running no yes 117.99 GiB 101.40 GiB 16.60 GiB
glusterpool running no yes 29.40 GiB 44.23 MiB 29.36 GiB
After:
$ virsh pool-list
Name State Autostart
-------------------------------------------
boot-scratch active no
default active no
glusterpool active no
$ virsh pool-list --details
Name State Autostart Persistent Capacity Allocation Available
---------------------------------------------------------------------------------
boot-scratch running no yes 117.99 GiB 101.40 GiB 16.60 GiB
default running no yes 117.99 GiB 101.40 GiB 16.60 GiB
glusterpool running no yes 29.40 GiB 44.23 MiB 29.36 GiB
There were two separate places with that were stringifying type of a
volume. One of the places was out of sync with types implemented
upstream.
To avoid such problems in the future, this patch adds a common function
to convert the type to string and reuses it across the two said places.
Add an extra space before the first column as we have when listing
domains.
Previous output:
$ virsh vol-list glusterpool
Name Path
-----------------------------------------
asdf gluster://gluster-node-1/gv0/asdf
c gluster://gluster-node-1/gv0/c
cd gluster://gluster-node-1/gv0/cd
$ virsh vol-list glusterpool --details
Name Path Type Capacity Allocation
----------------------------------------------------------------------
asdf gluster://gluster-node-1/gv0/asdf unknown 0.00 B 0.00 B
c gluster://gluster-node-1/gv0/c unknown 16.00 B 16.00 B
cd gluster://gluster-node-1/gv0/cd unknown 0.00 B 0.00 B
New output:
$ virsh vol-list glusterpool
Name Path
------------------------------------------------------------------------------
asdf gluster://gluster-node-1/gv0/asdf
c gluster://gluster-node-1/gv0/c
cd gluster://gluster-node-1/gv0/cd
$ virsh vol-list glusterpool --details
Name Path Type Capacity Allocation
------------------------------------------------------------------------
asdf gluster://gluster-node-1/gv0/asdf unknown 0.00 B 0.00 B
c gluster://gluster-node-1/gv0/c unknown 16.00 B 16.00 B
cd gluster://gluster-node-1/gv0/cd unknown 0.00 B 0.00 B
The 'vcpucount' command is a getter command for the vCPUu count. When
one or more of the filtering flags are specified the command returns the
value only for the selected combination. In this case the --live and
--config combination isn't valid. This however didn't cause errors as
the combination of flags was rejected by the libvirt API but then the
fallback code kicked in and requested the count in a way where the clash
of the flags didn't matter.
Mark the flag combination mutually exclusive so that users aren't
confused.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1024245
Allow adjust the number of commands to remember in the command
history.
* tools/virsh.c (vshReadlineInit): Read and sanity the
VIRSH_HISTSIZE variable.
(VIRSH_HISTSIZE_MAX): New constant.
* tools/virsh.pod: Document VIRSH_HISTSIZE variable.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit e962a57 added 'attach-disk --shareable', even though we
already had 'attach-disk --mode=shareable'. Worse, if the user
types 'attach-disk --mode=readonly --shareable', we create
non-sensical XML. The best solution is just to undocument the
duplicate spelling, by having it fall back to the preferred
spelling.
* tools/virsh-domain.c (cmdAttachDisk): Let alias handling fix our
mistake in exposing a second spelling for an existing option.
* tools/virsh.pod: Fix documentation.
Signed-off-by: Eric Blake <eblake@redhat.com>
We want to treat 'attach-disk --shareable' as an undocumented
alias for 'attach-disk --mode=shareable'. By improving our
alias handling, we can allow all such --bool -> --opt=value
replacements, and guarantee up front that the alias is not
mixed with its replacement.
* tools/virsh.c (vshCmddefOptParse, vshCmddefGetOption): Add
support for expanding bool alias to --opt=value.
(opts_echo): Add another alias to test it.
* tests/virshtest.c (mymain): Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
In commit b46c4787dd I changed the code to
watch long running jobs in virsh. Unfortunately I didn't take into
account that poll may get a hangup if the terminal is not a TTY and will
be closed.
This patch avoids polling the STDIN fd when there's no TTY.
Unconditional use of getenv is not secure in setuid env.
While not all libvirt code runs in a setuid env (since
much of it only exists inside libvirtd) this is not always
clear to developers. So make all the code paranoid, even
if it only ever runs inside libvirtd.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virt-login-shell binary shouldn't need to execute programs
relying on $PATH, but just in case set a fixed $PATH value
of /bin:/usr/bin
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The libvirt.so library has far too many library deps to allow
linking against it from setuid programs. Those libraries can
do stuff in __attribute__((constructor) functions which is
not setuid safe.
The virt-login-shell needs to link directly against individual
files that it uses, with all library deps turned off except
for libxml2 and libselinux.
Create a libvirt-setuid-rpc-client.la library which is linked
to by virt-login-shell. A config-post.h file allows this library
to disable all external deps except libselinux and libxml2.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We don't want to inherit any FDs in the new namespace
except for the stdio FDs. Explicitly close them all,
just in case some do not have the close-on-exec flag
set.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
'--print-xml' option is very useful for doing some test.
But we had to specify a real domain for it.
This patch could enable us to specify a fake domain
when using --print-xml option.
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The parameter allows overriding default listen address for '-incoming'
cmd line argument on destination.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Make it much easier to test a configuration built without readline
support, by reusing our existing library probe machinery. It gets
a bit tricky with readline, which does not provide a pkg-config
snippet, and which on some platforms requires one of several
terminal libraries as a prerequiste, but the end result should be
the same default behavior but now with the option to disable things.
* m4/virt-readline.m4 (LIBVIRT_CHECK_READLINE): Simplify by using
LIBVIRT_CHECK_LIB.
* tools/virsh.c: Convert USE_READLINE to WITH_READLINE.
Signed-off-by: Eric Blake <eblake@redhat.com>
'make distcheck' fails from a directory configured --without-lxc:
GEN virt-login-shell.1
Can't write-open ../../tools/virt-login-shell.1: Permission denied at /usr/bin/pod2man line 69.
* tools/Makefile.am (EXTRA_DIST): Ship pre-built man page.
Signed-off-by: Eric Blake <eblake@redhat.com>
It's possible to create a domain which will only use a TLS port
and will not have a non-TLS port set by using:
<graphics type='spice' autoport='yes' defaultMode='secure'/>
In such a setup, the 'graphics' node for the running domain will be:
<graphics type='spice' tlsPort='5900'
autoport='yes' listen='127.0.0.1'
defaultMode='secure'>
However, cmdDomDisplay loops over all the 'graphics' node, and it
ignores nodes which don't have a 'port' attribute. This means
'virsh domdisplay' will only return an empty string for domains
as the one above.
This commit looks for both 'port' and 'tlsPort' before deciding
to ignore a graphics node. It also makes sure 'port' is not printed
when it's not set.
This makes 'virsh domdisplay' return
'spice://127.0.0.1?tls-port=5900' for domains using only a TLS
port.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
After commit 8aecd35126 it'll detect
that a required option is not defined and it will assert and exit with:
virsh.c:1364: vshCommandOpt: Assertion `valid->name' failed.
Problem has been latent since commit ed23b106.
Signed-off-by: Eric Blake <eblake@redhat.com>
