The VIR_FREE() macro will cast away any const-ness. This masked a
number of places where we passed a 'const char *' string to
VIR_FREE. Fortunately in all of these cases, the variable was not
in fact const data, but a heap allocated string. Fix all the
variable declarations to reflect this.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Recent patches to fix handling of Ctrl-C when interacting with
ssh are not portable to mingw, which lacks termios handling.
The simplest solution is to just compile that code out, and
if someone ever appears that has a serious interest in getting
virsh fully functional even with ssh connections, they can
provide patches at that time.
* tools/virsh.h (_vshControl): Make termattr conditional.
* tools/virsh.c (vshTTYIsInterruptCharacter)
(vshTTYDisableInterrupt, vshTTYRestore, cfmakeraw, vshTTYMakeRaw)
(main): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Automake has builtin support to prevent botched conditional nesting,
but only if you use:
if FOO
else !FOO
endif !FOO
An example error message when using the wrong name:
daemon/Makefile.am:378: error: else reminder (LIBVIRT_INIT_SCRIPT_SYSTEMD_TRUE) incompatible with current conditional: LIBVIRT_INIT_SCRIPT_SYSTEMD_FALSE
daemon/Makefile.am:381: error: endif reminder (LIBVIRT_INIT_SCRIPT_SYSTEMD_TRUE) incompatible with current conditional: LIBVIRT_INIT_SCRIPT_SYSTEMD_FALSE
As our makefiles tend to have quite a bit of nested conditionals,
it's better to take advantage of the benefits of the build system
double-checking that our conditionals are well-nested, but that
requires a syntax check to enforce our usage style.
Alas, unlike C preprocessor and spec files, we can't use indentation
to make it easier to see how deeply nesting goes.
* cfg.mk (sc_makefile_conditionals): New rule.
* daemon/Makefile.am: Enforce the style.
* gnulib/tests/Makefile.am: Likewise.
* python/Makefile.am: Likewise.
* src/Makefile.am: Likewise.
* tests/Makefile.am: Likewise.
* tools/Makefile.am: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The vshWatchJob function registers a SIGINT handler that is used to
abort the active job and does not terminate virsh. Unfortunately, this
breaks when using the ssh transport as SIGINT is sent to the foreground
process group including the ssh transport processes which terminate.
This breaks the connection and migration is left in a insane state.
With this patch the terminal is modified to ignore key binding that
sends SIGINT and does the handling manually.
Resoves: https://bugzilla.redhat.com/show_bug.cgi?id=983348
This patch adds instrumentation to allow modification of config of the
terminal in virsh and successful reset of the state afterwards.
The added helpers allow to disable receiving of SIGINT when pressing the
key sequence (Ctrl+C usualy). This normally sends SIGINT to the
foreground process group which kills ssh processes used for transport of
the data.
When virBufferError is ok in cmdAttachDisk, the latter
should 'goto cleanup', instead of returning a false to
prevent memory leaking.
Signed-off-by: Eric Blake <eblake@redhat.com>
I noticed from an ./autobuild.sh run that we were installing a
virt-login-shell.exe binary when cross-building for mingw,
even though such a binary is necessarily worthless since the
code depends on lxc which is a Linux-only concept.
* tools/Makefile.am (conf_DATA, bin_PROGRAMS, dist_man1_MANS):
Make virt-login-shell installation conditional.
Signed-off-by: Eric Blake <eblake@redhat.com>
Noticed while reviewing another patch that had an accidental
mismatch due to refactoring. An audit of the code showed that
very few callers of vshCommandOpt were expecting a return of
-2, indicating programmer error, and of those that DID check,
they just propagated that status to yet another caller that
did not check. Fix this by making the code blatantly warn
the programmer, rather than silently ignoring it and possibly
doing the wrong thing downstream.
I know that we frown on assert()/abort() inside libvirtd
(libraries should NEVER kill the program that linked them),
but as virsh is an app rather than the library, and as this
is not the first use of assert() in virsh, I think this
approach is okay.
* tools/virsh.h (vshCommandOpt): Drop declaration.
* tools/virsh.c (vshCommandOpt): Make static, and add a
parameter. Abort on programmer errors rather than making callers
repeat that logic.
(vshCommandOptInt, vshCommandOptUInt, vshCommandOptUL)
(vshCommandOptString, vshCommandOptStringReq)
(vshCommandOptLongLong, vshCommandOptULongLong)
(vshCommandOptBool): Adjust callers.
Signed-off-by: Eric Blake <eblake@redhat.com>
Surprisingly, augtool get (or print) returns "path = value" while we are
only interested in the value. We need to remove the "path = " part from
the augtool's output. The following is an example of the augtool command
as used in virt-sanlock-cleanup script:
$ augtool get /files/etc/libvirt/qemu-sanlock.conf/disk_lease_dir
/files/etc/libvirt/qemu-sanlock.conf/disk_lease_dir = /var/lib/libvirt/sanlock
Commit a0b6a36f "fixed" what abfff210 broke (URI precedence), but
there was still one more thing missing to fix. When using virsh
parameters to setup debugging, those weren't honored, because at the
time debugging was initializing, arguments weren't parsed yet. To
make ewerything work as expected, we need to initialize the debugging
twice, once before debugging (so we can debug option parsing properly)
and then again after these options are parsed.
As a side effect, this patch also fixes a leak when virsh is ran with
multiple '-l' parameters.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Since 785ff34bf8 we are using the outputStr variable in cleanup label.
However, there is a possibility to jump to the label before the variable
has been declared:
virsh-pool.c: In function 'cmdPoolList':
virsh-pool.c:1121:25: error: jump skips variable initialization [-Werror=jump-misses-init]
goto asprintf_failure;
^
virsh-pool.c:1308:1: note: label 'asprintf_failure' defined here
asprintf_failure:
^
virsh-pool.c:1267:11: note: 'outputStr' declared here
char *outputStr = NULL;
VIR_FREE(caps) is not enough to free an array allocated
by vshStringToArray.
==17== 4 bytes in 1 blocks are definitely lost in loss record 4 of 728
==17== by 0x4EFFC44: virStrdup (virstring.c:554)
==17== by 0x128B10: _vshStrdup (virsh.c:125)
==17== by 0x129164: vshStringToArray (virsh.c:218)
==17== by 0x157BB3: cmdNodeListDevices (virsh-nodedev.c:409)
https://bugzilla.redhat.com/show_bug.cgi?id=1001536
==23== 41 bytes in 1 blocks are definitely lost in loss record 626 of 727
==23== by 0x4F0099F: virAsprintfInternal (virstring.c:358)
==23== by 0x15D2C9: cmdPoolList (virsh-pool.c:1268)
https://bugzilla.redhat.com/show_bug.cgi?id=1001536
virsh secret-list leak when no secrets are defined:
==27== 8 bytes in 1 blocks are definitely lost in loss record 6 of 726
==27== by 0x4E941DD: virAllocN (viralloc.c:183)
==27== by 0x5037F1A: remoteConnectListAllSecrets (remote_driver.c:3076)
==27== by 0x5004EC6: virConnectListAllSecrets (libvirt.c:16298)
==27== by 0x15F813: vshSecretListCollect (virsh-secret.c:397)
==27== by 0x15F0E1: cmdSecretList (virsh-secret.c:532)
And so do some other *-list commands.
https://bugzilla.redhat.com/show_bug.cgi?id=1001536
The messages were only freed on error.
==12== 1,100 bytes in 1 blocks are definitely lost in loss record 698 of 729
==12== by 0x4E98C22: virBufferAsprintf (virbuffer.c:294)
==12== by 0x12C950: vshOutputLogFile (virsh.c:2440)
==12== by 0x12880B: vshError (virsh.c:2254)
==12== by 0x131957: vshCommandOptDomainBy (virsh-domain.c:109)
==12== by 0x14253E: cmdStart (virsh-domain.c:3333)
https://bugzilla.redhat.com/show_bug.cgi?id=1001536
virsh cpu-stats guest --start 0 --count 3
It outputs right but the return value is 1 rather than 0
echo $?
1
Found by running libvirt-autotest
./run -t libvirt --tests virsh_cpu_stats
Commit abfff210 changed the order of vshParseArgv() and vshInit() in
order to make fix debugging of parameter parsing. However, vshInit()
did a vshReconnect() even though ctl->name wasn't set according to the
'-c' parameter yet. In order to keep both issues fixed, I've split
the vshInit() into vshInitDebug() and vshInit().
One simple memleak of ctl->name is fixed as a part of this patch,
since it is related to the issue it's fixing.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=999323
When using virsh secret-list - if the secret types are cephx or iscsi,
then allow fetch/print of the usage information. Prior to the change
the following would print:
UUID Usage
-----------------------------------------------------------
1b40a534-8301-45d5-b1aa-11894ebb1735 Unused
a5ba3efe-6adf-4a6a-b243-f010a043e314 Unused
Afterwards:
UUID Usage
-----------------------------------------------------------
1b40a534-8301-45d5-b1aa-11894ebb1735 ceph ceph_example
a5ba3efe-6adf-4a6a-b243-f010a043e314 iscsi libvirtiscsi
Use the new semantics of vshStringToArray to avoid leaking the array of
volumes to be deleted. The array would be leaked in case the first
volume was found in the domain definition. Also refactor the code a bit
to sanitize naming of variables hoding arrays and dimensions of the
arrays.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=996050
At a slightly larger memory expense allow stealing of items from the
string array returned from vshStringToArray and turn the result into a
string list compatible with virStringSplit. This will allow to use the
common dealloc function.
This patch also fixes a few forgotten checks of return from
vshStringToArray and one memory leak.
We were failing to autoprobe which schema to use for several
top-level XML elements.
* tools/virt-xml-validate.in (TYPE): Recognize <domainsnapshot>,
<filter>, and <secret>.
Signed-off-by: Eric Blake <eblake@redhat.com>
All good tools should have --help and --version output :)
Furthermore, we want to ensure a failed exit if xmllint fails,
or even for 'virt-xml-validate > /dev/full'.
* tools/virt-xml-validate.in: Add option parsing. Output errors
to stderr. Update documentation to match.
* tools/Makefile.am (virt-xml-validate): Substitute version.
Signed-off-by: Eric Blake <eblake@redhat.com>
Currently the virConnectBaselineCPU API does not expose the CPU features
that are part of the CPU's model. This patch adds a new flag,
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, that causes the API to explicitly
list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@intel.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=997765
==1349431== 8 bytes in 1 blocks are definitely lost in loss record 11 of 760
==1349431== at 0x4C2A554: calloc (vg_replace_malloc.c:593)
==1349431== by 0x4E9AA3E: virAllocN (in /usr/lib64/libvirt.so.0.1001.1)
==1349431== by 0x4EF28C4: virXPathNodeSet (in /usr/lib64/libvirt.so.0.1001.1)
==1349431== by 0x130B83: cmdCPUBaseline (in /usr/bin/virsh)
==1349431== by 0x12C608: vshCommandRun (in /usr/bin/virsh)
==1349431== by 0x12889A: main (in /usr/bin/virsh)
Address a number of code, style and docs issues identified
in review of virt-login-shell after it was merged.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To avoid having to assign a failure code to the returned variable switch
this function to negative logic. This will fix issue with invalid number
of cpus returning success return code.
https://bugzilla.redhat.com/show_bug.cgi?id=996466
I attempted 'virsh blockcopy $dom vda $path --wait --verbose', then
hit Ctrl-C; I was a bit surprised to see this error message:
Block Copy: [ 3 %]error: failed to query job for disk vda
when I had been expecting:
Block Copy: [ 3 %]
Copy aborted
* tools/virsh-domain.c (cmdBlockCopy): Print graceful exit message
rather than error when ctrl-c interrupts job.
Signed-off-by: Eric Blake <eblake@redhat.com>
The virLoginShellAllowedUser method must not free the 'groups'
parameter it is given, as that is owned by the caller.
The virLoginShellAllowedUser method should be checking
'!*ptr' (ie empty string) rather than '!ptr' (NULL string)
since the latter cannot be true.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
virt-login-shell.c was failing to compile with
CC virt_login_shell-virt-login-shell.o
virt-login-shell.c: In function 'main':
virt-login-shell.c:205:5: error: implicit declaration of function 'setlocale' [-Werror=implicit-function-declaration]
virt-login-shell.c:205:5: error: nested extern declaration of 'setlocale' [-Werror=nested-externs]
virt-login-shell.c:205:20: error: 'LC_ALL' undeclared (first use in this function)
Add a virt-login-shell binary that can be set as a user's
shell, such that when they login, it causes them to enter
the LXC container with a name matching their user name.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The VIR_DOMAIN_PAUSED_GUEST_PANICKED constant is badly named,
leaking the QEMU event name. Elsewhere in the API we use
'CRASHED' rather than 'PANICKED', and the addition of 'GUEST'
is redundant since all events are guest related.
Thus rename it to VIR_DOMAIN_PAUSED_CRASHED, which matches
with VIR_DOMAIN_RUNNING_CRASHED and VIR_DOMAIN_EVENT_CRASHED.
It was added in commit 14e7e0ae8d
which post-dates v1.1.0, so is safe to rename before 1.1.1
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The VIR_DOMAIN_SHUTDOWN_CRASHED state constant does not appear
to be used in the QEMU code anyway. It also doesn't make much
(any) sense, since the 'shutdown' state is a transient state
between 'running' and 'shutoff' and when a guest crashes, it
does not end up in a 'shutdown' state, only 'shutoff'.
It was added in commit 14e7e0ae8d
which post-dates v1.1.0, so is safe to remove before 1.1.1
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Noticed that the expected "not supported" error is dropped when
invoking 'virsh snapshot-list dom' on a Xen installation running
the libxl driver
virsh snapshot-list test
error: Invalid snapshot: virDomainSnapshotFree
The error is overwritten by a call to virDomainSnapshotFree
in cleanup code within cmdSnapshotList. Prevent overwritting
the real error by not calling virDomainSnapshotFree with a NULL
virDomainSnapshotPtr.
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=923053
When cdrom is block type, the virsh change-media failed to insert
source info because virsh uses "<source block='/dev/sdb'/>" while
the correct name of the attribute for block disks is "dev".
Add a "--pass-fds N,M,..." arg to the virsh start/create
methods. This allows pre-opened file descriptors from the
shell to be passed on into the guest
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Use the virDomainSetMemoryStatsPeriodFlags() to pass a period defined by
usage of a new --period option in order to set the collection period for the
balloon driver. This may enable or disable the collection based on the value.
Add the --current, --live, & --config options to dommemstat.
Recent changes uncovered FORWARD_NULL and NEGATIVE_RETURNS problems with
the processing of the 'ndevices' and its associated allocated arrays in
'vshNodeDeviceListCollect' due to the possibility of returning -1 in a
call and using the returned value as a for loop index end condition.
Recent changes uncovered FORWARD_NULL and NEGATIVE_RETURNS problems with
the processing of the 'nActiveIfaces' and 'nInactiveIfaces' and their
associated allocated arrays in 'vshInterfaceListCollect' due to the
possibility of returning -1 in a call and using the return value as a
for loop index end condition.
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Actually, I'm turning this function into a macro as filename,
function name and line number needs to be passed. The new
function virAsprintfInternal is introduced with the extended set
of arguments.
Free the old XML strings before overwriting them if the user
has chosen to reedit the file or force the redefinition.
Found by Alex Jia trying to reproduce another bug:
https://bugzilla.redhat.com/show_bug.cgi?id=977430#c3
Paolo Bonzini pointed out that it's actually possible to migrate a qemu
instance that was paused due to I/O error and it will be able to work on
the destination if the storage is accessible.
This patch introduces flag VIR_MIGRATE_ABORT_ON_ERROR that cancels the
migration in case an I/O error happens while it's being performed and
allows migration without this flag. This flag can be possibly used for
other error reasons that may be introduced in the future.
This patch fixes changes done in commit 29c1e913e4
that was pushed without implementing review feedback.
The flag introduced by the patch is changed to VIR_DOMAIN_VCPU_GUEST and
documentation makes the difference between regular hotplug and this new
functionality more explicit.
The virsh options that enable the use of the new flag are changed to
"--guest" and the documentation is fixed too.
This flag will allow to use qemu guest agent commands to disable
(offline) and enable (online) processors in a live guest that has the
guest agent running.
Our documentation says a pool may be referenced by its name or UUID
anywhere if it makes sense (pool-name and pool-uuid are the only
exceptions). However, vol-create and vol-create-as commands did not obey
this.
Because it's a valid combination. p2p still uses a separate channel
for qemu migration, so there's value in letting the user specify a manual
migrate URI for overriding auto-port, or libvirt's FQDN lookup.
What _isn't_ allowed is --migrateuri and TUNNELLED, since there is
no separate migration channel. Disallow that instead
I noticed several unusual spacings in for loops, and decided to
fix them up. See the next commit for the syntax check that found
all of these.
* examples/domsuspend/suspend.c (main): Fix spacing.
* python/libvirt-override.c: Likewise.
* src/conf/interface_conf.c: Likewise.
* src/security/virt-aa-helper.c: Likewise.
* src/util/virconf.c: Likewise.
* src/util/virhook.c: Likewise.
* src/util/virlog.c: Likewise.
* src/util/virsocketaddr.c: Likewise.
* src/util/virsysinfo.c: Likewise.
* src/util/viruuid.c: Likewise.
* src/vbox/vbox_tmpl.c: Likewise.
* src/xen/xen_hypervisor.c: Likewise.
* tools/virsh-domain-monitor.c (vshDomainStateToString): Drop
default case, to let compiler check us.
* tools/virsh-domain.c (vshDomainVcpuStateToString): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Introduced by commit 1daa4ba33a. vshCommandOptStringReq returns
0 on *success* or the option is not required && not present, both
are right result. Error out when returning 0 is not correct.
the caller, it doesn't have to check wether it
Don't print 'OPTION' if there's no options. Just behaves as DESCRIPTION
does.
This mostly affects 'interface' command group.
Signed-off-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
Reported-by: Li Yang <liyang.fnst@cn.fujitsu.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Automake already passes all CFLAGS to the linker too, so it
is not necessary to set WARN_LDFLAGS in addition to the
WARN_CFLAGS variable.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
These all existed before virfile.c was created, and for some reason
weren't moved.
This is mostly straightfoward, although the syntax rule prohibiting
write() had to be changed to have an exception for virfile.c instead
of virutil.c.
This movement pointed out that there is a function called
virBuildPath(), and another almost identical function called
virFileBuildPath(). They really should be a single function, which
I'll take care of as soon as I figure out what the arglist should look
like.
Recent commit '53531e16' resulted in a new Coverity warning regarding
a missing break in the ':' options processing. Adjust the commit to
avoid the issue.
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
Mention file/volume contents instead of just 'file'/'volume'.
Also change Download->download in vol-download help,
to be consistent with other volume commands.
https://bugzilla.redhat.com/show_bug.cgi?id=955537
For long options, print:
* the option as specified by the user if it's unknown
* the canonical long option if its argument is not
a number (and should be)
And for missing arguments, print both the short and
the long option name.
(Doing only one of those would require either parsing
argv ourselves or let getopt print the errors, since
we can't tell long and short options apart by optopt
or longindex)
https://bugzilla.redhat.com/show_bug.cgi?id=949373
Unsupported long option:
$ virsh --pm
Before:
error: unsupported option '-
After:
error: unsupported option '--pm'. See --help.
Missing parameter:
$ virsh --deb
Before:
error: option '-d' requires an argument
After:
error: option '-d'/'--debug' requires an argument
$ virsh -rd
Before:
error: option '-d' requires an argument
After:
error: option '-d'/'--debug' requires an argument
Non-numeric parameter:
$ virsh --deb duck
Before:
error: option -d takes a numeric argument
After:
error: option --debug takes a numeric argument
'virsh help | grep nodedev-det' shows only nodedev-detach, but
'virsh help nodedev | grep nodedev-det' also shows the old alias
nodedev-dettach that we intentionally hid in commit af3f9aab.
See also commit 787f4fe and this bug report:
https://bugzilla.redhat.com/show_bug.cgi?id=956966
* tools/virsh.c (vshCmdGrpHelp): Copy suppression of vshCmdHelp.
Signed-off-by: Eric Blake <eblake@redhat.com>
The virsh nodedev-detach command has a new --driver option. If it's
given virsh will attempt to use the new virNodeDeviceDetachFlags API
instead of virNodeDeviceDettach. Validation of the driver name string
is left to the hypervisor (qemu accepts "kvm" or "vfio". The only
other hypervisor that implements these functions is xen, and it only
accepts NULL).
This patch factors out the vCPU count retrieval including fallback means
into vshCPUCountCollect() and removes the duplicated code to retrieve
individual counts.
The --current flag (this flag is assumed by default) now works also with
--maximum or --active without the need to explicitly specify the state
of the domain that is requested.
This patch also fixes the output of "virsh vcpucount domain" on inactive
domains:
Before:
$ virsh vcpucount domain
maximum config 4
error: Requested operation is not valid: domain is not running
current config 4
error: Requested operation is not valid: domain is not running
After:
$virsh vcpucount domain
maximum config 4
current config 4
.. and for transient domains too:
Before:
$ virsh vcpucount transient-domain
error: Requested operation is not valid: cannot change persistent config of a transient domain
maximum live 3
error: Requested operation is not valid: cannot change persistent config of a transient domain
current live 1
After:
$ virsh vcpucount transient-domain
maximum live 3
current live 1
Using of a incorrect value for the --holdtime option was silently
ignored and 0 was used. In case a negative number was used, it
overflowed as the API expects a unsigned int.
Fix the data type and getter function type and report errors on
incorrect values.
