* tools/virsh.c: avoid double free of domain, when weight value of blkiotune
less than 0, codes will free domain and jump to cleanup section, however,
cleanup will free domain again.
Detected in valgrind run:
==21297== ERROR SUMMARY: 20 errors from 20 contexts (suppressed: 69 from 8)
==21297==
==21297== 1 errors in context 1 of 20:
==21297== Invalid read of size 4
==21297== at 0x40E209B: virDomainFree (libvirt.c:2096)
==21297== by 0x8065274: cmdBlkiotune (virsh.c:3695)
==21297== by 0x8054CC1: vshCommandRun (virsh.c:13135)
==21297== by 0x806B967: main (virsh.c:14487)
==21297== Address 0x446ad48 is 0 bytes inside a block of size 36 free'd
==21297== at 0x4005B0A: free (vg_replace_malloc.c:325)
==21297== by 0x406814D: virFree (memory.c:310)
==21297== by 0x40D6635: virReleaseDomain (datatypes.c:243)
==21297== by 0x40D6C5E: virUnrefDomain (datatypes.c:280)
==21297== by 0x40E20B9: virDomainFree (libvirt.c:2101)
==21297== by 0x8065297: cmdBlkiotune (virsh.c:3613)
==21297== by 0x8054CC1: vshCommandRun (virsh.c:13135)
==21297== by 0x806B967: main (virsh.c:14487)
==21297==
==21297==
==21297== 1 errors in context 2 of 20:
==21297== Invalid read of size 4
==21297== at 0x40E1FE6: virDomainFree (libvirt.c:2092)
==21297== by 0x8065274: cmdBlkiotune (virsh.c:3695)
==21297== by 0x8054CC1: vshCommandRun (virsh.c:13135)
==21297== by 0x806B967: main (virsh.c:14487)
==21297== Address 0x446ad48 is 0 bytes inside a block of size 36 free'd
==21297== at 0x4005B0A: free (vg_replace_malloc.c:325)
==21297== by 0x406814D: virFree (memory.c:310)
==21297== by 0x40D6635: virReleaseDomain (datatypes.c:243)
==21297== by 0x40D6C5E: virUnrefDomain (datatypes.c:280)
==21297== by 0x40E20B9: virDomainFree (libvirt.c:2101)
==21297== by 0x8065297: cmdBlkiotune (virsh.c:3613)
==21297== by 0x8054CC1: vshCommandRun (virsh.c:13135)
==21297== by 0x806B967: main (virsh.c:14487)
* how to reproduce?
% valgrind -v --leak-check=full virsh blkiotune guestname --weight -1
"optional" is not a very good meta-syntactic construct in our man
page. I scrubbed this, and additionally improved some documentation
on mutually exclusive options. For example,
[[--live] [--config] | [--current]]
implies a set of optional flags, where within the set you can have
either --current or a choice of 0, 1, or both --live and --config.
* tools/virsh.pod: Use "[name]" rather than "optional name" for
optional arguments.
When libvirtd restarts it will attempt to reconnect to existing
LXC containers. If it loads a XML state file for the container
the container will appear running. If we fail to read the PID
file, or fail to connect to the LXC monitor, we should be killing
off the guest, but if the VMs cgroup does not exist any more,
cleanup will get skipped. Reading the PID file is also pointless
since the PID is in the XML statefile
In lxcReconnectVM we do not need to read the PID file. If part
of the reconnect process fails we need to run the VM terminate
code as a safety net.
In lxcVMTerminate, if we can't obtain the VM cgroup, we know
the process has died, but we must still run lxcVMCleanup to
clear out the virDomainObjPtr live state
* src/lxc/lxc_driver.c: Fix cleanup of dead VMs on restart
These typos are introduced by file renaming in commit b17b4afaf.
src/remote/qemu_protocol.x \
src/remote/remote_protocol.x \
src/rpc/gendispatch.pl:
s/remote_generator/gendispatch/
src/rpc/genprotocol.pl:
s/remote\/remote_protocol/remote_protocol/
The regression is introduced by Commit da1eba6b, the new
codes with this commit doesn't reset "ret" to "-1" when
it fails on parsing the device XML (live device attachment)
This patch changes the codes to reset the "ret" and "-1",
and also changes the codes so that it don't modify "ret"
for condition checking.
How to reproduce:
% cat test.xml
<disk type='oops' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/test.img'/>
<target dev='vda' bus='virtio'/>
</disk>
% virsh attach-device $domain test.xml
Device attached successfully
The device attachment failed actually with error "unknown disk type 'oops'",
however, it reports success.
As long as we guarantee RPC struct layout stability, we might as
well also guarantee RPC enum value constancy.
* src/Makefile.am (r1, r2, PDWTAGS): Adjust rule to pick up named
and anonymous enums.
* src/remote_protocol-structs: Add enum values.
* src/qemu_protocol-structs: Likewise.
* src/virnetprotocol-structs: Likewise.
Enforce the recent flags cleanups - we want to use 'unsigned int flags'
in any of our APIs (except where backwards compatibility is important,
in the public migration APIs), and that all flags are checked for
validity (except when there are stub functions that completely
ignore the flags argument).
There are a few minor tweaks done here to avoid false positives:
signed arguments passed to open() are renamed oflags, and flags
arguments that are legitimately ignored are renamed flags_unused.
* cfg.mk (sc_flags_usage): New rule.
(exclude_file_name_regexp--sc_flags_usage): And a few exemptions.
(sc_flags_debug): Tweak wording.
* src/util/iohelper.c (runIO, main): Rename variable.
* src/util/util.c (virSetInherit): Likewise.
* src/fdstream.h (virFDStreamOpenFile, virFDStreamCreateFile):
Likewise.
* src/fdstream.c (virFDStreamOpenFileInternal)
(virFDStreamOpenFile, virFDStreamCreateFile): Likewise.
* src/util/command.c (virExecWithHook) [WIN32]: Likewise.
* src/util/util.c (virFileOpenAs, virDirCreate) [WIN32]: Likewise.
* src/locking/lock_manager.c (virLockManagerPluginNew)
[!HAVE_DLFCN_H]: Likewise.
* src/locking/lock_driver_nop.c (virLockManagerNopNew)
(virLockManagerNopAddResource, virLockManagerNopAcquire)
(virLockManagerNopRelease, virLockManagerNopInquire): Likewise.
Silently ignored flags get in the way of new features that
use those flags.
Regarding ESX migration flags - right now, ESX silently enforces
VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE, and
VIR_MIGRATE_LIVE, even if those flags were not supplied; it ignored
other flags. This patch does not change the implied bits (it permits
but does not require them), but enforces only the supported bits.
If further cleanup is needed to be more particular about migration
flags, that should be a separate patch.
* src/esx/esx_device_monitor.c (esxDeviceOpen): Reject unknown
flags.
* src/esx/esx_driver.c (esxOpen, esxDomainReboot)
(esxDomainXMLFromNative, esxDomainXMLToNative)
(esxDomainMigratePrepare, esxDomainMigratePerform)
(esxDomainMigrateFinish): Likewise.
* src/esx/esx_interface_driver.c (esxInterfaceOpen): Likewise.
* src/esx/esx_network_driver.c (esxNetworkOpen): Likewise.
* src/esx/esx_nwfilter_driver.c (esxNWFilterOpen): Likewise.
* src/esx/esx_secret_driver.c (esxSecretOpen): Likewise.
* src/esx/esx_storage_driver.c (esxStorageOpen): Likewise.
The documentation for vshCommandOptString claims that it returns
-1 on a missing required argument, but in reality, that error
message was unreachable (it was buried inside an if clause that
is true only if the argument was present). The code was so hairy
that I decided a rewrite would make it easier to understand,
and actually return the error values we want.
Meanwhile, our construction guarantees that all vshCmdOpt have
a non-null def member, so there are some redundant checks that
can be trimmed.
* tools/virsh.c (vshCommandOpt): Alter signature.
(vshCommandOptInt, vshCommandOptUInt, vshCommandOptUL)
(vshCommandOptString, vshCommandOptLongLong)
(vshCommandOptULongLong, vshCommandOptBool): Adjust all callers.
(vshCommandOptArgv): Remove dead condition.
Commit 461e0f1a broke migration, because there was a code path
that tried to enable an internal flag while still going through
the public function. Split the internal flag into a separate
callback, and validate that flags do not overlap.
* src/conf/domain_conf.c (virDomainDefFormat): Split...
(virDomainDefFormatInternal): ...to separate the flag check.
(virDomainObjFormat): Adjust caller.
The "libvirt supports:" section on the main page of libvirt.org
contains a list of hypervisors with links that point to the sites of
the underlying virt technologies. The entry for KVM points to
http://www.linux-kvm.org/, for example. People coming to libvirt.org
for the first time are likely to know about those sites, and they're
probably interested in how libvirt manages those technologies. This
patch points those links to the libvirt driver pages instead. It also
consolidates KVM and QEMU as there is only one libvirt driver page for
them. Finally, it adds a line about networking support.
v2: incorporate Eric's feedback adding project links to driver pages.
website: Add project links to KVM/QEMU driver page
website: Add project links to Xen driver page
website: Add project links to LXC driver page
website: Add project links to OpenVZ driver page
website: Add project links to UML driver page
website: Add project links to Virtualbox driver page
website: Add project links to ESX driver page
website: Add project links to VMware driver page
Commit f548480b broke migration v3 on qemu, because the driver
passed flags on through to qemu_migration even though
qemu_migration wasn't using those flags.
* src/qemu/qemu_migration.h (QEMU_MIGRATION_FLAGS): New define.
* src/qemu/qemu_driver.c: Simplify all migration callbacks.
* src/qemu/qemu_migration.c (qemuMigrationConfirm): Fix regression.
The previous patches only cleaned up ATTRIBUTE_UNUSED flags cases;
auditing the drivers found other places where flags was being used
but not validated. In particular, domainGetXMLDesc had issues with
clients accepting a different set of flags than the common
virDomainDefFormat helper function.
* src/conf/domain_conf.c (virDomainDefFormat): Add common flag check.
* src/uml/uml_driver.c (umlDomainAttachDeviceFlags)
(umlDomainDetachDeviceFlags): Reject unknown
flags.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc)
(vboxDomainAttachDeviceFlags)
(vboxDomainDetachDeviceFlags): Likewise.
* src/qemu/qemu_driver.c (qemudDomainMemoryPeek): Likewise.
(qemuDomainGetXMLDesc): Document common flag handling.
* src/libxl/libxl_driver.c (libxlDomainGetXMLDesc): Likewise.
* src/lxc/lxc_driver.c (lxcDomainGetXMLDesc): Likewise.
* src/openvz/openvz_driver.c (openvzDomainGetXMLDesc): Likewise.
* src/phyp/phyp_driver.c (phypDomainGetXMLDesc): Likewise.
* src/test/test_driver.c (testDomainGetXMLDesc): Likewise.
* src/vmware/vmware_driver.c (vmwareDomainGetXMLDesc): Likewise.
* src/xenapi/xenapi_driver.c (xenapiDomainGetXMLDesc): Likewise.
This adds four options for virsh command attach-disk.
--cache option allows user to specify cache mode of disk device
from virsh command line when attaching a disk device.
--serial option allows user to specify serial string of disk device
from virsh command line when attaching a disk device.
--shareable option allows user to specify whether the disk device is
shareable between domains when attaching a disk device from virsh
command line.
--address option allows user to specify address of disk device when
attaching a disk device.
If the server succesfully validates the client cert, it will send
back a single byte, under TLS. If it fails, it will close the
connection. In this case, we were just reporting the standard
I/O error. The original RPC code had a special case hack for the
GNUTLS_E_UNEXPECTED_PACKET_LENGTH error code to make us report
a more useful error message
* src/rpc/virnetclient.c: Return ENOMSG if we get
GNUTLS_E_UNEXPECTED_PACKET_LENGTH
* src/rpc/virnettlscontext.c: Report cert failure if we
see ENOMSG
Many volume operations will fail if the volume in question is being
allocated. These operations were returning VIR_ERR_INTERNAL_ERROR
when they should be returning VIR_ERR_OPERATION_INVALID.
This patch adds the --current option to "virsh setvcpus"
command. Currently "virsh setvcpus" command supports
"--live" and "--config" , but "--current" option.
From view of consistency, it's reasonable to support
"--current" option too.
When --current is specified, it affects a "current"
domain.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
This patch extends qemudDomainSetVcpusFlags() function to support
VIR_DOMAIN_AFFECT_CURRENT flag.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
This patch extends virDomainSetVcpusFlags API to support
VIR_DOMAIN_AFFECT_CURRENT flag.
Now because most APIs accept VIR_DOMAIN_AFFECT_CURRENT flags,
virDomainSetVcpusFlags API should also do.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
Rather than trying to clean up the ssh child ourselves, and risk
subtle differences from the socket creation error path, we can
just use the new APIs.
* src/rpc/virnetsocket.c (virNetSocketFree): Use new function.
By requesting the pid in virCommandRunAsync, fdstream was claiming
that it would manually wait for the process. But on the failure
path, the child process was being leaked.
* src/fdstream.c (virFDStreamOpenFileInternal): Auto-reap child.
When using virCommandRunAsync and saving the pid for later, it
is useful to be able to reap that pid in the same way that it
would have been auto-reaped by virCommand if we had passed
NULL for the pid argument in the first place.
* src/util/command.c (virPidWait, virPidAbort): New functions,
created from...
(virCommandWait, virCommandAbort): ...bodies of these.
(includes): Drop duplicate <stdlib.h>. Ensure that our pid_t
assumptions hold.
(virCommandRunAsync): Improve documentation.
* src/util/command.h (virPidWait, virPidAbort): New prototypes.
* src/libvirt_private.syms: Export them.
* docs/internals/command.html.in: Document them.
In the Ubuntu development release we recently got a new udev that
moves /var/run to /run, /var/lock to /run/lock and /dev/shm to /run/shm.
This change in udev requires updating the apparmor security driver in
libvirt[1].
Attached is a patch that:
* adjusts src/security/virt-aa-helper.c to allow both
LOCALSTATEDIR/run/libvirt/**/%s.pid and /run/libvirt/**/%s.pid. While
the profile is not as precise, LOCALSTATEDIR/run/ is typically a symlink
to /run/ anyway, so there is no additional access (remember that
apparmor resolves symlinks, which is why this is still required even
if /var/run points to /run).
* adjusts example/apparmor/libvirt-qemu paths for /dev/shm
[1]https://launchpad.net/bugs/810270
--
Jamie Strandboge | http://www.canonical.com
Without this patch, the make rule in a VPATH build was trying to
invoke ../../docs/../../docs/todo.pl, which didn't exist.
* docs/Makefile.am (todo.html.in): Using $< already implies
$(srcdir) in GNU make VPATH situations.
Similar to the recent qemu_protocol-structs addition.
* src/virnetprotocol-structs: New file.
* src/Makefile.am (%_protocol-structs): Factor body...
(PDWTAGS): ...into new helper macro.
(virnetprotocol-structs): New rule.
(PROTOCOL_STRUCTS): Add virnetprotocol-structs.
We disable some drivers when building without libvirtd in configure,
but we do not do the same thing in libvirt.spec. It may break rpm
building without libvirtd.
Getting metadata on storage allocates a memory (path) which need to
be freed after use otherwise it gets leaked. This means after use of
virStorageFileGetMetadataFromFD or virStorageFileGetMetadata one
must call virStorageFileFreeMetadata to free it. This function frees
structure internals and structure itself.
When qemuMonitorCloseFileHandle is called in error path, we need to
preserve the original error since a possible further error when running
closefd monitor command is not very useful to users.
When creating new qemu process we saved domain status XML only after the
process was fully setup and running. In case libvirtd was killed before
the whole process finished, once libvirtd started again it didn't know
anything about the new process and we end up with an orphaned qemu
process. Let's save the domain status XML as soon as we know the PID so
that libvirtd can kill the process on restart.
The compiler might optimize based on our declaration that something
is unused. Putting that declaration in the header risks getting
out of sync with the actual implementation, so it belongs better
only in the .c files. We were mostly compliant, and a new syntax
check will help us in the future.
* cfg.mk (sc_avoid_attribute_unused_in_header): New syntax check.
* src/nodeinfo.h (nodeGetCPUStats, nodeGetMemoryStats): Delete
attribute already present in .c file.
* src/qemu/qemu_domain.h (qemuDomainEventFlush): Likewise.
* src/util/virterror_internal.h (virReportErrorHelper): Parameters
are actually used by .c file.
* src/xenxs/xen_sxpr.h (xenFormatSxprDisk): Adjust prototype.
* src/xenxs/xen_sxpr.c (xenFormatSxprDisk): Delete unused argument.
(xenFormatSxpr): Adjust caller.
* src/xen/xend_internal.c (xenDaemonAttachDeviceFlags)
(xenDaemonUpdateDeviceFlags): Likewise.
Suggested by Daniel Veillard.
For static functions not used as callbacks, there's no need to
keep an unused parameter.
* src/conf/domain_conf.c (virDomainChrDefParseTargetXML)
(virDomainTimerDefParseXML, virDomainHostdevSubsysUsbDefParseXML)
(virDomainVcpuPinDefParseXML): Drop unused parameter.
(virDomainChrDefParseXML, virDomainDefParseXML)
(virDomainHostdevDefParseXML): Update callers.
(virDomainNetDefParseXML): Mark flags used.
Valid loglevel range for virsh is 0-4. Update virsh man page
accordingly. Also explain virsh ENV variables and values.
Signed-off-by: Supriya Kannery <supriyak@in.ibm.com>
The last patch breaks make check for two reasons. First, it reverses the
condition but leaves default level unchanged, so instead of not printing
anything but errors before the patch it now prints all debug messages by
default. Second, you forgot to change -d5 option passed to virsh in
tests/virsh-optparse to -d0; the script wants to see all debug messages.
Aligning loglevel values of virsh to that of libvirt.
"DEBUG"=0 loglevel, when specified through commandline or
env variable, should log all the messages. "ERROR=4"
should log only error messages.
Signed-off-by: Supriya Kannery <supriyak@in.ibm.com>
In 2f4d2496a8 I didn't notice that one
part of virFileOpenAs doesn't actually call to virFileOpenAsNoFork but
rather includes a copy of the code from there.
No need to repeat common code.
* bootstrap.conf (gnulib_modules): Import calloc-posix.
* src/util/bridge.c (brInit): Use virSetCloseExec.
(brSetInterfaceUp): Adjust flags name.
* src/uml/uml_driver.c (umlSetCloseExec): Delete.
(umlStartVMDaemon): Use util version instead.
'unsigned a' and 'unsigned int a' are synonyms, but we generally
always spell out the 'int' in that case. Fixing this will avoid
a false positive in the next syntax-check commit.
* src/conf/node_device_conf.h (pci_config_address)
(_virNodeDevCapsDef): Prefer 'unsigned int' over 'unsigned'.
