Old gcc complains about shadowing 'sync' variable:
../../src/qemu/qemu_agent.c: In function 'qemuAgentSetTime':
../../src/qemu/qemu_agent.c:1737: warning: declaration of 'sync'
shadows a global declaration [-Wshadow]
/usr/include/unistd.h:464: warning: shadowed declaration is here
[-Wshadow]
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The commit 84c59ffa improved the way we change ejectable media.
If for any reason the first "eject" didn't open the tray we
should return with error.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This partially reverts commits b279e52f7 and ea18f8b2.
It turns out our code base is full of:
if ((struct.member = virBlahFromString(str)) < 0)
goto error;
Meanwhile, the C standard says it is up to the compiler whether
an enum is signed or unsigned when all of its declared values
happen to be positive. In my testing (Fedora 20, gcc 4.8.2),
the compiler picked signed, and nothing changed. But others
testing with gcc 4.7 got compiler warnings, because it picked
the enum to be unsigned, but no unsigned value is less than 0.
Even worse:
if ((struct.member = virBlahFromString(str)) <= 0)
goto error;
is silently compiled without warning, but incorrectly treats -1
from a bad parse as a large positive number with no warning; and
without the compiler's help to find these instances, it is a
nightmare to maintain correctly. We could force signed enums
with a dummy negative declaration in each enum, or cast the
result of virBlahFromString back to int after assigning to an
enum value, or use a temporary int for collecting results from
virBlahFromString, but those actions are all uglier than what we
were trying to cure by directly using enum types for struct
values in the first place. It's better off to just live with int
members, and use 'switch ((virFoo) struct.member)' where we want
the compiler to help, than to track down all the conversions from
string to enum and ensure they don't suffer from type problems.
* src/util/virstorageencryption.h: Revert back to int declarations
with comment about enum usage.
* src/util/virstoragefile.h: Likewise.
* src/conf/domain_conf.c: Restore back to casts in switches.
* src/qemu/qemu_driver.c: Likewise.
* src/qemu/qemu_command.c: Add cast rather than revert.
Signed-off-by: Eric Blake <eblake@redhat.com>
For internal structs, we might as well be type-safe and let the
compiler help us with less typing required on our part (getting
rid of casts is always nice). In trying to use enums directly,
I noticed two problems in virstoragefile.h that can't be fixed
without more invasive refactoring: virStorageSource.format is
used as more of a union of multiple enums in storage volume
code (so it has to remain an int), and virStorageSourcePoolDef
refers to pooltype whose enum is declared in src/conf, but where
src/util can't pull in headers from src/conf.
* src/util/virstoragefile.h (virStorageNetHostDef)
(virStorageSourcePoolDef, virStorageSource): Use enums instead of
int for fields of internal types.
* src/qemu/qemu_command.c (qemuParseCommandLine): Cover all values.
* src/conf/domain_conf.c (virDomainDiskSourceParse)
(virDomainDiskSourceFormat): Simplify clients.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskInternal): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
One caveat though, qemu-ga is expecting time and returning time
in nanoseconds. With all the buffering and propagation delay, the
time is already wrong once it gets to the qemu-ga, but there's
nothing we can do about it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Coverity complains about event being leaked in
qemuDomainCheckRemoveOptionalDisk. The best fix for it is to remove the
disk directly since we already know its index.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this
commit are related to storage (storage_conf) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
When qemu driver is polling for migration to finish (in
qemuMigrationWaitForCompletion), it may happen that another job allowed
during migration is running and if it does not finish within 30 seconds,
migration would be cancelled because of that. However, we can just
ignore the timeout and let the waiting loop try again later.
If an event fired at the end of migration is ever implemented in QEMU,
we can just wait for the event instead of polling for migration status
and libvirt will behave consistently, i.e., migration won't be cancelled
in case another job started during migration takes long time to finish.
For bug https://bugzilla.redhat.com/show_bug.cgi?id=1083238
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
As a side effect, the return value of qemuDomainObjEnterMonitorAsync is
not directly used as the return value of qemuProcess{Start,Stop}CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
If job queue is full or waiting for a job times out, the function
returns -2 so that it can be handled in a different way by callers.
The change is safe since all existing callers of
qemuDomainObjBeginNestedJob check the return value to be less than zero.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
If the compression program for external snapshot memory image isn't
found we exitted the function without terminating the domain job. This
caused the domain to be unusable.
The problem was introduced in commit 7df5093f.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097503
Move sharable PCI handling functions to domain_addr.[ch], and
change theirs prefix from 'qemu' to 'vir':
- virDomainPCIAddressAsString;
- virDomainPCIAddressBusSetModel;
- virDomainPCIAddressEnsureAddr;
- virDomainPCIAddressFlagsCompatible;
- virDomainPCIAddressGetNextSlot;
- virDomainPCIAddressReleaseSlot;
- virDomainPCIAddressReserveAddr;
- virDomainPCIAddressReserveNextSlot;
- virDomainPCIAddressReserveSlot;
- virDomainPCIAddressSetFree;
- virDomainPCIAddressSetGrow;
- virDomainPCIAddressSlotInUse;
- virDomainPCIAddressValidate;
The only change here is function names, the implementation itself
stays untouched.
Extract common allocation code from DomainPCIAddressSetCreate
into virDomainPCIAddressSetAlloc.
Introduce new files (domain_addr.[ch]) to provide
an API for domain device handling that could be
shared across the drivers.
A list of data types were extracted and moved there:
qemuDomainPCIAddressBus -> virDomainPCIAddressBus
qemuDomainPCIAddressBusPtr -> virDomainPCIAddressBusPtr
_qemuDomainPCIAddressSet -> virDomainPCIAddressSet
qemuDomainPCIAddressSetPtr -> virDomainPCIAddressSetPtr
qemuDomainPCIConnectFlags -> virDomainPCIConnectFlags
Also, move the related definitions and macros.
https://bugzilla.redhat.com/show_bug.cgi?id=1002813
If qemuDomainBlockResize() is passed a size not on a KiB boundary - that
is passed a size based in bytes (VIR_DOMAIN_BLOCK_RESIZE_BYTES), then
depending on the source format (qcow2 or qed), the value passed must
be on a sector (or 512 byte) boundary. Since other libvirt code quietly
adjusts the capacity values, then do so here as well.
With this patch, virDomainFSFreeze will pass the mountpoints argument
to qemu guest agent. For example,
virDomainFSFreeze(dom, {"/mnt/vol1", "/mnt/vol2"}, 2, 0)
will issue qemu guest agent command:
{"execute":"guest-fsfreeze-freeze",
"arguments":{"mountpoints":["/mnt/vol1","/mnt/vol2"]}}
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
Acked-by: Daniel P. Berrange <berrange@redhat.com>
Use qemuDomainSnapshotFSFreeze() and qemuDomainSnapshotFSFThaw() which are
already implemented for snapshot quiescing.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
"Freezed" is not an English word.
* src/lxc/lxc_driver.c (lxcFreezeContainer): Fix typo.
* src/qemu/qemu_driver.c (qemuDomainSnapshotFSFreeze): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1002813
If qemuDomainBlockResize() is passed a size not on a KiB boundary - that
is passed a size based in bytes (VIR_DOMAIN_BLOCK_RESIZE_BYTES), then
depending on the source format (qcow2 or qed), the value passed must
be on a sector (or 512 byte) boundary. Since other libvirt code quietly
adjusts the capacity values, then do so here as well - of course ensuring
that adjustment still fits.
Signed-off-by: John Ferlan <jferlan@redhat.com>
QEMU commit 5e2ac51 added a boolean '-msg timestamp=[on|off]'
option, which can enable timestamps on errors:
$ qemu-system-x86_64 -msg timestamp=on zghhdorf
2014-04-09T13:25:46.779484Z qemu-system-x86_64: -msg timestamp=on: could
not open disk image zghhdorf: Could not open 'zghhdorf': No such file or
directory
Enable this timestamp if the QEMU binary supports it.
Add a 'log_timestamp' option to qemu.conf for disabling this behavior.
Adds 'quiesced' status into qemuDomainObjPrivate that tracks whether
FSFreeze is requested in the domain.
It modifies error code from qemuDomainSnapshotFSFreeze and
qemuDomainSnapshotFSThaw, so that a caller can know whether the command is
actually sent to the guest agent. If the error is caused before sending a
freeze command, a counterpart thaw command shouldn't be sent either, not to
confuse fsfreeze status tracking.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This uses the new QEMU_CAPS_HOST_PCI_MULTIDOMAIN capability when
present, for -devivce pci-assign, -device vfio-pci, and -pcidevice.
While creating tests for this new functionality, I noticed that the
xmls for two existing tests had erroneously specified an
until-now-ignored domain="0x0002", so I corrected those two tests, and
also added two failure tests to be sure that we alert users who
attempt to use a non-zero domain with a qemu that doesn't support it.
Quite a long time ago, (apparently between qemu 0.12 and 0.13) qemu
quietly began supporting the optional specification of a domain in the
host-side address of all pci passthrough commands (by simply
prepending it to the bus:slot.function format, as
"dddd:bb:ss.f"). Since machines with multiple PCI domains are very
rare, this never came up in practice, so libvirt was never updated to
support it.
This patch takes the first step to supporting specification of a non-0
domain in the host-side address of PCI devices being assigned to a
domain, by adding a capability bit to indicate support
"QEMU_CAPS_HOST_PCI_MULTIDOMAIN", and detect it. Since this support
was added in a version prior to the minimum version required for
QMP-style capabilities detection, the capability is always enabled for
any qemu that uses QMP for capabilities detection. For older qemus,
the only clue that a domain can be specified in the host pci address
is the presence of the string "[seg:]" in the help string for
-pcidevice. (Ironically, libvirt will not be modified to support
specification of domain for -pcidevice, since any qemu new enough for
us to care about also supports "-device pci-assign" or "-device
vfio-pci", which are greatly preferred).
In "src/util/" there are many enumeration (enum) declarations.
Sometimes, it's better using a typedef for variable types,
function types and other usages. Other enumeration will be
changed to typedef's in the future.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
A couple of places in the QEMU XML -> ARGV conversion code
raised an error but then forgot to return an error status
due to missing gotos. While fixing this also tweak style
of a couple of other error reports
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If a domain network interface that contains a <filterref> is modified
"live" using "virsh update-device --live", libvirtd would crash. This
was because the code supporting live update of an interface's
filterref was assuming that a filterref might be added or modified,
but didn't account for removing the filterref, resulting in a null
dereference of the filter name.
Introduced with commit 258fb278, which was first in libvirt v1.0.1.
This addresses https://bugzilla.redhat.com/show_bug.cgi?id=1093301
To avoid memory leak of the "backingStoreRaw" field when reparsing
backing chains a new function is being introduced by this patch that
shall be used to clear backing store information.
The memory leak was introduced in commit 8823272d41.
This commit provides the ability to virDomainMigrateToURI to
check for SASL credentials when attempts to migrate a domain
with the driver QEMU.
Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@cloudwatt.com>
When EIO comes to qemu while it's replying to
qemuMigrationUpdateJobStatus(), qemu blocks, the migration of RAM can
complete in the meantime, and when qemu unblocks, it sends us
BLOCK_IO_ERROR plus migrations "status": "complete". Even though we
act upon the BLOCK_IO_ERROR by setting the proper state of the domain,
the call still waits for the proper reply on monitor for query_migrate
and after it gets it, it checks that migration is completed and the
migration is finished. This is what abort_on_error flag was meant for
(we can migrate with these errors, but this flag must inhibit such
behaviour). Changing the order of the steps guarantees the flag works
properly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1045833
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Convert all remaining clients of readdir to use the new
interface, so that we can ensure (unlikely) errors while
reading a directory are reported.
* src/openvz/openvz_conf.c (openvzAssignUUIDs): Use new
interface.
* src/parallels/parallels_storage.c (parallelsFindVolumes)
(parallelsFindVmVolumes): Report readdir failures.
* src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Ignore readdir
failures.
* src/secret/secret_driver.c (loadSecrets): Likewise.
* src/qemu/qemu_hostdev.c
(qemuHostdevHostSupportsPassthroughVFIO): Report readdir failures.
* src/xen/xen_inotify.c (xenInotifyOpen): Likewise.
* src/xen/xm_internal.c (xenXMConfigCacheRefresh): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
When a snapshot operation finishes we have to recheck the backing chain
of all disks involved in the snapshot. And we need to do that even if
the operation failed because some of the disks might have changed if
QEMU did not support transactions.
The check for a network being active during interface attach was being
done individually in several places (by both the lxc driver and the
qemu driver), but those places were too specific, leading to it *not*
being checked when allocating a connection/device from a macvtap or
hostdev network.
This patch puts a single check in networkAllocateActualDevice(), which
is always called before the any network interface is attached to any
type of domain. It also removes all the other now-redundant checks
from the lxc and qemu drivers.
NB: the following patches are prerequisites for this patch, in the
case that it is backported to any branch:
440beeb network: fix virNetworkObjAssignDef and persistence
8aaa5b6 network: create statedir during driver initialization
b9e9549 network: change location of network state xml files
411c548 network: set macvtap/hostdev networks active if their state
file exists
This fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=880483
When a VM fails to launch due to error creating nwfilter
rules, we must avoid overwriting the original error when
tearing down the partially created rules.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit c4206d7 fixed the overflow for running domains. However, we need
a similar check when setting migration speed on inactive domains.
At first look, it may seem the check in c4206d7 is now redundant but
qemuDomainMigrateSetMaxSpeed is not the only caller of
qemuMonitorSetMigrationSpeed so we need to check the bandwidth in both
places.
https://bugzilla.redhat.com/show_bug.cgi?id=1083483
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Each backing store of a given disk is associated with a unique index
(which is also formatted in domain XML) for easier addressing of any
particular backing store. With this patch, any backing store can be
addressed by its disk target and the index. For example, "vdc[4]"
addresses the backing store with index equal to 4 of the disk identified
by "vdc" target. Such shorthand can be used in any API in place for a
backing file path:
virsh blockcommit domain vda --base vda[3] --top vda[2]
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
virStorageFileChainLookup is able to give use virStorageSourcePtr which
contains the pointer to its canonical path. Let's use a more general
virStorageSourcePtr instead of just canonical path.
Former base_canon maps to baseSource->path.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
virStorageFileChainLookup is able to give use virStorageSourcePtr which
contains the pointer to its canonical path. There's no need for the
caller to store both of them.
Former top_meta maps to topSource and top_canon maps to topSource->path.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Recent discussions around naming of 'pci' vs 'pci.0' for PPC
made me go back and look at the PPC emulator in every historical
version of QEMU since 1.0. The results were worse than I imagined.
This patch adds the logic required to make libvirt work with PPC
correctly with naming variations across all versions & machine
types.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To avoid having the root of a backing chain present twice in the list we
need to invert the working of virStorageFileGetMetadataRecurse.
Until now the recursive worker created a new backing chain element from
the name and other information passed as arguments. This required us to
pass the data of the parent in a deconstructed way and the worker
created a new entry for the parent.
This patch converts this function so that it just fills in metadata
about the parent and creates a backing chain element from those. This
removes the duplication of the first element.
To avoid breaking the test suite, virstoragetest now calls a wrapper
that creates the parent structure explicitly and pre-fills it with the
test data with same function signature as previously used.
Switch over to storing of the backing chain as a recursive
virStorageSource structure.
This is a string based move. Currently the first element will be present
twice in the backing chain as currently the retrieval function stores
the parent in the newly detected chain. This will be fixed later.
Remove the obsolete field replaced by data in "path".
The testsuite requires tweaking as the name of the backing file is now
stored one layer deeper in the backing chain linked list.
Currently, libvirt is using legacy USB controller as default. There
are problems with VGA which can't work correctly with USB Keyboard and
USB Mouse.
While providing -nodefaults, ppc64 should be specifying the usb
controller explicitly in place of using the legacy
controller(-usb). Qemu spapr initialization code when sees "-usb" adds a
USB Keyboard and USB Mouse by default. And libvirt has added a USB
keyboard and USB mouse.
A recent fix in the in qemu VGA code uncoverd this problem, which
resulted in addition of extra keyboard and mouse to the qemu machine.
This patch is to set pci-ohci as USB default controller.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
Remove the pointer from def->cputune.vcpupin after unplugging
the CPU and also free the bitmap contained in the structure
by calling virDomainVcpuPinDel instead of VIR_FREE.
Introduced by commit 0df1a79.
This makes virDomainLookupVcpuPin redundant.
https://bugzilla.redhat.com/show_bug.cgi?id=1088165
When passing migration bandwidth to QEMU, we multiply it by 1024 * 1024
to convert the speed to B/s and the result still needs to fit in
int64_t.
https://bugzilla.redhat.com/show_bug.cgi?id=1083483
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The original chain lookup code had to pass in the starting name,
because it was not available in the chain. But now that we have
added fields to the struct, this parameter is redundant.
* src/util/virstoragefile.h (virStorageFileChainLookup): Alter
signature.
* src/util/virstoragefile.c (virStorageFileChainLookup): Adjust
handling of top of chain.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Adjust caller.
* tests/virstoragetest.c (testStorageLookup, mymain): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The chain lookup function was inconsistent on whether it left
a message in the log when looking up a name that is not found
on the chain (leaving a message for OOM or if name was
relative but not part of the chain), and could litter the log
even when successful (when name was relative but deep in the
chain, use of virFindBackingFile early in the chain would complain
about a file not found). It's easier to make the function
consistently emit a message exactly once on failure, and to let
all callers rely on the clean semantics.
* src/util/virstoragefile.c (virStorageFileChainLookup): Always
report error on failure. Simplify relative lookups.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Avoid
overwriting error.
Signed-off-by: Eric Blake <eblake@redhat.com>
Currently, virCgroupGetPercpuStats is only used by the LXC driver,
filling out the CPUTIME stats. qemuDomainGetPercpuStats does this
and also filles out VCPUTIME stats.
Extend virCgroupGetPercpuStats to also report VCPUTIME stats if
nvcpupids is non-zero. In the LXC driver, we don't have cpupids.
In the QEMU driver, there is at least one cpupid for a running domain,
so the behavior shouldn't change for QEMU either.
Also rename getSumVcpuPercpuStats to virCgroupGetPercpuVcpuSum.
Refactor the function to avoid multiple wrappers splitting identical
fields from the now common metadata struct.
The refactor is done by folding in the wrapper used for disk sources
which allows us to lookup secrets via the secret driver. This may allow
using stored secrets for snapshot disk images too in the future.
Now that we store all metadata about a storage image in a
virStorageSource struct let's use it also to store information needed by
the storage driver to access and do operations on the files.
Since it is an abbreviation, PCI should always be fully
capitalized or full lower case, never Pci.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since it is an abbreviation, USB should always be fully
capitalized or full lower case, never Usb.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since it is an abbreviation, SCSI should always be fully
capitalized or full lower case, never Scsi.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
I almost wrote a hash value free function that just called
VIR_FREE, then realized I couldn't be the first person to
do that. Sure enough, it was worth factoring into a common
helper routine.
* src/util/virhash.h (virHashValueFree): New function.
* src/util/virhash.c (virHashValueFree): Implement it.
* src/util/virobject.h (virObjectFreeHashData): New function.
* src/libvirt_private.syms (virhash.h, virobject.h): Export them.
* src/nwfilter/nwfilter_learnipaddr.c (virNWFilterLearnInit): Use
common function.
* src/qemu/qemu_capabilities.c (virQEMUCapsCacheNew): Likewise.
* src/qemu/qemu_command.c (qemuDomainCCWAddressSetCreate):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorGetBlockInfo): Likewise.
* src/qemu/qemu_process.c (qemuProcessWaitForMonitor): Likewise.
* src/util/virclosecallbacks.c (virCloseCallbacksNew): Likewise.
* src/util/virkeyfile.c (virKeyFileParseGroup): Likewise.
* tests/qemumonitorjsontest.c
(testQemuMonitorJSONqemuMonitorJSONGetBlockInfo): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
nmdm is a FreeBSD driver which allows to create a pair of tty
devices one of which is passed to the guest and second is used
by the client.
This patch adds new 'nmdm' character device type. Its definition
looks this way:
<serial type='nmdm'>
<source master='/dev/nmdm0A' slave='/dev/nmdm0B'/>
</serial>
Master is passed to the hypervisior and slave is used for client
connection.
Also implement domainOpenConsole() for bhyve driver based on that.
Right now, virStorageFileMetadata tracks bool backingStoreIsFile
for whether the backing string specified in metadata can be
resolved as a file (covering both block and regular file
resources) or is treated as a network protocol. But when
merging this struct with virStorageSource, it will be easier
to just actually track which type of resource it is, as well
as have a reserved value for the case where the resource type
is unknown (or had an error during probing).
* src/util/virstoragefile.h (virStorageType): Add a placeholder
value, swap order to match similar public enum.
* src/util/virstoragefile.c (virStorage): Update string mapping.
* src/conf/domain_conf.c (virDomainDiskSourceParse)
(virDomainDiskDefParseXML, virDomainDiskDefFormat)
(virDomainDiskSourceFormat): Adjust clients.
* src/conf/snapshot_conf.c (virDomainSnapshotDiskDefParseXML):
Likewise.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskExternalOverlayInactive)
(qemuDomainSnapshotPrepareDiskInternal)
(qemuDomainSnapshotCreateSingleDiskActive): Likewise.
* src/qemu/qemu_command.c (qemuGetDriveSourceString): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit 5b3492fa aimed to fix this and caught one error but exposed
another one. When agent command is being executed and the thread
waiting for the reply is woken up by an event (e.g. EOF in case of
shutdown), the command finishes with no data (rxObject == NULL), but
no error is reported, since this might be desired by the caller
(e.g. suspend through agent). However, in other situations, when the
data are required (e.g. getting vCPUs), we proceed to getting desired
data out of the reply, but none of the virJSON*() functions works well
with NULLs. I chose the way of a new parameter for qemuAgentCommand()
function that specifies whether reply is required and behaves
according to that.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1058149
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Every caller checked the return value and logged an error
- one if no device with the specified MAC was found,
other if there were multiple devices matching the MAC address
(except for qemuDomainUpdateDeviceConfig which logged the same
message in both cases).
Move the error reporting into virDomainNetFindIdx, since in both cases,
we couldn't find one single match - it's just the error messages that
differ.
On all the places where qemuAgentComand() was called, we did a check
for errors in the reply. Unfortunately, some of the places called
qemuAgentCheckError() without checking for non-null reply which might
have resulted in a crash.
So this patch makes the error-checking part of qemuAgentCommand()
itself, which:
a) makes it look better,
b) makes the check mandatory and, most importantly,
c) checks for the errors if and only if it is appropriate.
This actually fixes a potential crashers when qemuAgentComand()
returned 0, but reply was NULL. Having said that, it *should* fix the
following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1058149
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Now that we have a common struct, it's time to start using it!
Since external snapshots make a longer backing chain, it is
only natural to use the same struct for the file created by
the snapshot as what we use for <domain> disks.
* src/conf/snapshot_conf.h (_virDomainSnapshotDiskDef): Use common
struct instead of open-coded duplicate fields.
* src/conf/snapshot_conf.c (virDomainSnapshotDiskDefClear)
(virDomainSnapshotDiskDefParseXML, virDomainSnapshotAlignDisks)
(virDomainSnapshotDiskDefFormat)
(virDomainSnapshotDiskGetActualType): Adjust clients.
* src/qemu/qemu_conf.c (qemuTranslateSnapshotDiskSourcePool):
Likewise.
* src/qemu/qemu_driver.c (qemuDomainSnapshotDiskGetSourceString)
(qemuDomainSnapshotCreateInactiveExternal)
(qemuDomainSnapshotPrepareDiskExternalOverlayActive)
(qemuDomainSnapshotPrepareDiskExternal)
(qemuDomainSnapshotPrepare)
(qemuDomainSnapshotCreateSingleDiskActive): Likewise.
* src/storage/storage_driver.c
(virStorageFileInitFromSnapshotDef): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Move some functions out of domain_conf for use in the next
patch where snapshot starts to directly use structs in
virstoragefile.
* src/conf/domain_conf.c (virDomainDiskDefFree)
(virDomainDiskSourcePoolDefParse): Adjust callers.
(virDomainDiskSourceDefClear, virDomainDiskSourcePoolDefFree)
(virDomainDiskAuthClear): Move...
* src/util/virstoragefile.c (virStorageSourceClear)
(virStorageSourcePoolDefFree, virStorageSourceAuthClear): ...and
rename.
* src/conf/domain_conf.h (virDomainDiskAuthClear): Drop
declaration.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Adjust
caller.
* src/util/virstoragefile.h: Declare them.
* src/libvirt_private.syms (virstoragefile.h): Export them.
Signed-off-by: Eric Blake <eblake@redhat.com>
The code in virstoragefile.c is getting more complex as I
consolidate backing chain handling code. But for the setuid
virt-login-shell, we don't need to crawl backing chains. It's
easier to audit things for setuid security if there are fewer
files involved, so this patch moves the one function that
virFileOpen() was actually relying on to also live in virfile.c.
* src/util/virstoragefile.c (virStorageFileIsSharedFS)
(virStorageFileIsSharedFSType): Move...
* src/util/virfile.c (virFileIsSharedFS, virFileIsSharedFSType):
...to here, and rename.
(virFileOpenAs): Update caller.
* src/security/security_selinux.c
(virSecuritySELinuxSetFileconHelper)
(virSecuritySELinuxSetSecurityAllLabel)
(virSecuritySELinuxRestoreSecurityImageLabelInt): Likewise.
* src/security/security_dac.c
(virSecurityDACRestoreSecurityImageLabelInt): Likewise.
* src/qemu/qemu_driver.c (qemuOpenFileAs): Likewise.
* src/qemu/qemu_migration.c (qemuMigrationIsSafe): Likewise.
* src/util/virstoragefile.h: Adjust declarations.
* src/util/virfile.h: Likewise.
* src/libvirt_private.syms (virfile.h, virstoragefile.h): Move
symbols as appropriate.
Signed-off-by: Eric Blake <eblake@redhat.com>
This one is a relatively easy move. We don't ever convert the
enum to or from strings (it is inferred from other elements in
the xml, rather than directly represented).
* src/conf/domain_conf.h (virDomainDiskSecretType): Move...
* src/util/virstoragefile.h (virStorageSecreteType): ...and
rename.
* src/conf/domain_conf.c (virDomainDiskSecretType): Drop unused
enum conversion.
(virDomainDiskAuthClear, virDomainDiskDefParseXML)
(virDomainDiskDefFormat): Adjust clients.
* src/qemu/qemu_command.c (qemuGetSecretString): Likewise.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePoolAuth):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Another struct being moved to util. This one doesn't have as
much use yet, thankfully.
* src/conf/domain_conf.h (virDomainDiskSourcePoolMode)
(virDomainDiskSourcePoolDef): Move...
* src/util/virstoragefile.h (virStorageSourcePoolMode)
(virStorageSourcePoolDef): ...and rename.
* src/conf/domain_conf.c (virDomainDiskSourcePoolDefFree)
(virDomainDiskSourceDefClear, virDomainDiskSourcePoolDefParse)
(virDomainDiskDefParseXML, virDomainDiskSourceDefParse)
(virDomainDiskSourceDefFormatInternal)
(virDomainDiskDefForeachPath, virDomainDiskSourceIsBlockType):
Adjust clients.
* src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Likewise.
* src/libvirt_private.syms (domain_conf.h): Move symbols...
(virstoragefile.h): ...as appropriate.
Signed-off-by: Eric Blake <eblake@redhat.com>
In order to reuse the newly-created host-side disk struct in
the virstoragefile backing chain code, I first have to move
it to util/. This starts the process, by first moving the
security label structures.
* src/conf/domain_conf.h (virDomainDefGenSecurityLabelDef)
(virDomainDiskDefGenSecurityLabelDef, virSecurityLabelDefFree)
(virSecurityDeviceLabelDefFree, virSecurityLabelDef)
(virSecurityDeviceLabelDef): Move...
* src/util/virseclabel.h: ...to new file.
(virSecurityLabelDefNew, virSecurityDeviceLabelDefNew): Rename the
GenSecurity functions.
* src/qemu/qemu_process.c (qemuProcessAttach): Adjust callers.
* src/security/security_manager.c (virSecurityManagerGenLabel):
Likewise.
* src/security/security_selinux.c
(virSecuritySELinuxSetSecurityFileLabel): Likewise.
* src/util/virseclabel.c: New file.
* src/conf/domain_conf.c: Move security code, and fix fallout.
* src/Makefile.am (UTIL_SOURCES): Build new file.
* src/libvirt_private.syms (domain_conf.h): Move symbols...
(virseclabel.h): ...to new section.
Signed-off-by: Eric Blake <eblake@redhat.com>
While running qemucaps2xmltest, it was found that valgrind pointed out
the following memory leaks:
==29896== 0 bytes in 1 blocks are definitely lost in loss record 1 of 65
==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896== by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999)
==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789)
==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896== by 0x41FFD1: virtTestRun (testutils.c:201)
==29896== by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
==29896== by 0x42074D: virtTestMain (testutils.c:789)
==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==29896==
==29896== 0 bytes in 1 blocks are definitely lost in loss record 2 of 65
==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896== by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999)
==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789)
==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896== by 0x41FFD1: virtTestRun (testutils.c:201)
==29896== by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
==29896== by 0x42074D: virtTestMain (testutils.c:789)
==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
Signed-off-by: Eric Blake <eblake@redhat.com>
In all other drivers we are doing so. Moreover, we don't want to parse
runtime information in attach (even if the attach is meant as live)
because we are generating the runtime info ourselves. We can't trust
users they supply sane values anyway.
==1140== 9 bytes in 1 blocks are definitely lost in loss record 72 of 1,151
==1140== at 0x4A06C2B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1140== by 0x623C758: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
==1140== by 0x50FD763: virXMLPropString (virxml.c:483)
==1140== by 0x510F8B7: virDomainDeviceInfoParseXML (domain_conf.c:3685)
==1140== by 0x511ACFD: virDomainChrDefParseXML (domain_conf.c:7535)
==1140== by 0x5121D13: virDomainDeviceDefParse (domain_conf.c:9918)
==1140== by 0x13AE6313: qemuDomainAttachDeviceFlags (qemu_driver.c:6926)
==1140== by 0x13AE65FA: qemuDomainAttachDevice (qemu_driver.c:7005)
==1140== by 0x51C77DA: virDomainAttachDevice (libvirt.c:10231)
==1140== by 0x127FDD: remoteDispatchDomainAttachDevice (remote_dispatch.h:2404)
==1140== by 0x127EC5: remoteDispatchDomainAttachDeviceHelper (remote_dispatch.h:2382)
==1140== by 0x5241F81: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
When doing live attach, we are passing the inactive definition anyway
since we are passing the result of virDomainDeviceDefCopy() which does
inactive copy by default.
Moreover, we are doing the same mistake in qemuhotplugtest.
Just a side note - it makes perfect sense to parse the runtime info
like alias in qemuDomainDetachDevice and qemuDomainUpdateDeviceFlags()
as in some cases the only difference to distinguish two devices can be
just their alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch adds an element to QEMU's capability XML, to
show if the underlying QEMU binary supports the live disk
snapshotting or not.
This allows any client to know ahead of time if the feature
is available.
Without this information available, the only way to check
for the snapshot support is to request one and check for
errors.
Signed-off-by: Francesco Romani <fromani@redhat.com>
Currently, the Linux kernel treats values of '0' and '1' as
the minimum of 2. Values larger than the maximum are changed
to the maximum.
Re-reading the shares value after setting it reflects this in
the live domain XML.
Currently, <cputune><shares>0</shares></cputune> is treated
as if it were not specified.
Treat is as a valid value if it was explicitly specified
and write it to the cgroups.
qemuDomainSetSchedulerParametersFlags() calls virQEMUDriverGetConfig() twice
and makes the reference counter leak. This removes redundant call.
Problem introduced in commit 45ad1ad
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch adds qemuMonitorGetDumpGuestMemoryCapability, which is used to check
whether the specified dump-guest-memory format is supported by qemu.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
It's finally time to start tracking disk backing chains in
<domain> XML. The first step is to start refactoring code
so that we have an object more convenient for representing
each host source resource in the context of a single guest
<disk>. Ultimately, I plan to move the new type into src/util
where it can be reused by virStorageFile, but to make the
transition easier to review, this patch just creates the
new type then fixes everything until it compiles again.
* src/conf/domain_conf.h (_virDomainDiskDef): Split...
(_virDomainDiskSourceDef): ...to new struct.
(virDomainDiskAuthClear): Use new type.
* src/conf/domain_conf.c (virDomainDiskDefFree): Split...
(virDomainDiskSourceDefClear): ...to new function.
(virDomainDiskGetType, virDomainDiskSetType)
(virDomainDiskGetSource, virDomainDiskSetSource)
(virDomainDiskGetDriver, virDomainDiskSetDriver)
(virDomainDiskGetFormat, virDomainDiskSetFormat)
(virDomainDiskAuthClear, virDomainDiskGetActualType)
(virDomainDiskDefParseXML, virDomainDiskSourceDefFormat)
(virDomainDiskDefFormat, virDomainDiskDefForeachPath)
(virDomainDiskDefGetSecurityLabelDef)
(virDomainDiskSourceIsBlockType): Adjust all users.
* src/lxc/lxc_controller.c (virLXCControllerSetupDisk):
Likewise.
* src/lxc/lxc_driver.c (lxcDomainAttachDeviceMknodHelper):
Likewise.
* src/qemu/qemu_command.c (qemuAddRBDHost, qemuParseRBDString)
(qemuParseDriveURIString, qemuParseGlusterString)
(qemuParseISCSIString, qemuParseNBDString)
(qemuDomainDiskGetSourceString, qemuBuildDriveStr)
(qemuBuildCommandLine, qemuParseCommandLineDisk)
(qemuParseCommandLine): Likewise.
* src/qemu/qemu_conf.c (qemuCheckSharedDevice)
(qemuAddISCSIPoolSourceHost, qemuTranslateDiskSourcePool):
Likewise.
* src/qemu/qemu_driver.c (qemuDomainUpdateDeviceConfig)
(qemuDomainPrepareDiskChainElement)
(qemuDomainSnapshotCreateInactiveExternal)
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskInternal)
(qemuDomainSnapshotPrepare)
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotUndoSingleDiskActive)
(qemuDomainBlockPivot, qemuDomainBlockJobImpl)
(qemuDomainBlockCopy, qemuDomainBlockCommit): Likewise.
* src/qemu/qemu_migration.c (qemuMigrationIsSafe): Likewise.
* src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase)
(qemuProcessInitPasswords): Likewise.
* src/security/security_selinux.c
(virSecuritySELinuxSetSecurityFileLabel): Likewise.
* src/storage/storage_driver.c (virStorageFileInitFromDiskDef):
Likewise.
* tests/securityselinuxlabeltest.c (testSELinuxLoadDef):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
It's easy to shed the daemon these days. With this XML snippet:
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/some/dummy/path/test.bin'>
<seclabel model='dac' relabel='no'/>
</source>
<target dev='vdb' bus='virtio'/>
<readonly/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
</disk>
I get the SIGSEGV when starting the domain. The thing is, when
starting a domain, we check for its disk presence. For some reason,
when determining the disk chain, we parse the <seclabel/> (don't ask
me why). However, there's no label attribute in the XML, so we end up
calling virParseOwnershipIds() over NULL string:
[Switching to Thread 0x7ffff10c4700 (LWP 30956)]
__strchr_sse42 () at ../sysdeps/x86_64/multiarch/strchr.S:136
136 ../sysdeps/x86_64/multiarch/strchr.S: No such file or directory.
(gdb) bt
#0 __strchr_sse42 () at ../sysdeps/x86_64/multiarch/strchr.S:136
#1 0x00007ffff749f800 in virParseOwnershipIds (label=0x0, uidPtr=uidPtr@entry=0x7ffff10c2df0, gidPtr=gidPtr@entry=0x7ffff10c2df4) at util/virutil.c:2115
#2 0x00007fffe929f006 in qemuDomainGetImageIds (gid=0x7ffff10c2df4, uid=0x7ffff10c2df0, disk=0x7fffe40cb000, vm=0x7fffe40a6410, cfg=0x7fffe409ae00) at qemu/qemu_domain.c:2385
#3 qemuDomainDetermineDiskChain (driver=driver@entry=0x7fffe40120e0, vm=vm@entry=0x7fffe40a6410, disk=disk@entry=0x7fffe40cb000, force=force@entry=false) at qemu/qemu_domain.c:2414
#4 0x00007fffe929f128 in qemuDomainCheckDiskPresence (driver=driver@entry=0x7fffe40120e0, vm=vm@entry=0x7fffe40a6410, cold_boot=cold_boot@entry=true) at qemu/qemu_domain.c:2250
#5 0x00007fffe92b6fc8 in qemuProcessStart (conn=conn@entry=0x7fffd4000b60, driver=driver@entry=0x7fffe40120e0, vm=vm@entry=0x7fffe40a6410, migrateFrom=migrateFrom@entry=0x0, stdin_fd=stdin_fd@entry=-1, stdin_path=stdin_path@entry=0x0, snapshot=snapshot@entry=0x0,
vmop=vmop@entry=VIR_NETDEV_VPORT_PROFILE_OP_CREATE, flags=flags@entry=1) at qemu/qemu_process.c:3813
#6 0x00007fffe93087e8 in qemuDomainObjStart (conn=0x7fffd4000b60, driver=driver@entry=0x7fffe40120e0, vm=vm@entry=0x7fffe40a6410, flags=flags@entry=0) at qemu/qemu_driver.c:6051
#7 0x00007fffe9308e32 in qemuDomainCreateWithFlags (dom=0x7fffcc000d50, flags=0) at qemu/qemu_driver.c:6105
#8 0x00007ffff753c5cc in virDomainCreate (domain=domain@entry=0x7fffcc000d50) at libvirt.c:8861
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Busy enterprise workloads hosted on large sized VM's tend to dirty
memory faster than the transfer rate achieved via live guest migration.
Despite some good recent improvements (& using dedicated 10Gig NICs
between hosts) the live migration may NOT converge.
Recently support was added in qemu (version 1.6) to allow a user to
choose if they wish to force convergence of their migration via a
new migration capability : "auto-converge". This feature allows for qemu
to auto-detect lack of convergence and trigger a throttle-down of the
VCPUs.
This patch includes the libvirt support needed to trigger this
feature. (Testing is in progress)
Signed-off-by: Chegu Vinod <chegu_vinod@hp.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When checking compatibility of a device with a domain definition, we
should know what we're going to do with the device. Because we may need
to check for different things when we're attaching a new device versus
detaching an existing device.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
A device needs to be checked for compatibility with the domain
definition it corresponds to. Specifically, for VIR_DOMAIN_AFFECT_CONFIG
case we should check against persistent def rather than active def.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=844378
When qemu dies early after connecting to its monitor but before we
actually try to read something from the monitor, we would just fail
domain start with useless message:
"An error occurred, but the cause is unknown"
This is because the real error gets reported in a monitor EOF handler
executing within libvirt's event loop.
The fix is to take any error set in qemuMonitor structure and propagate
it into the thread-local error when qemuMonitorClose is called and no
thread-local error is set.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Wire up all the pieces to send arbitrary qemu events to a
client using libvirt-qemu.so. If the extra bookkeeping of
generating event objects even when no one is listening turns
out to be noticeable, we can try to further optimize things
by adding a counter for how many connections are using events,
and only dump events when the counter is non-zero; but for
now, I didn't think it was worth the code complexity.
* src/qemu/qemu_driver.c
(qemuConnectDomainQemuMonitorEventRegister)
(qemuConnectDomainQemuMonitorEventDeregister): New functions.
* src/qemu/qemu_monitor.h (qemuMonitorEmitEvent): New prototype.
(qemuMonitorDomainEventCallback): New typedef.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONIOProcessEvent):
Report events.
* src/qemu/qemu_monitor.c (qemuMonitorEmitEvent): New function, to
pass events through.
* src/qemu/qemu_process.c (qemuProcessHandleEvent): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Valgrind reported leaking of maxCpus and arch strings from
virXPathString, as well as the leak of the machineMaxCpus array.
Don't use 'str' for the strings we don't want to free, to allow
freeing of 'str' in the cleanup label and free machineMaxCpus
in virCapsReset too.
Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
As part of the goal to get away from doing string matching on
filenames when deciding whether to emit a log message, turn
the virLogSource enum into a struct which contains a log
"name". There will eventually be one virLogSource instance
statically declared per source file. To minimise churn in this
commit though, a single global instance is used.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The dtrace probe macros rely on the logging API. We can't make
the internal.h header include the virlog.h header though since
that'd be a circular include. Instead simply split the dtrace
probes into their own header file, since there's no compelling
reason for them to be in the main internal.h header.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We allow translation from no_bandwidth to has_bandwidth for a vnic.
However, going in the opposite direction is not implemented. It's not
limitation of the API rather than internal implementation. The problem
is, we correctly detect that user hasn't specified any outbound (say
he wants to clear out outbound). However, this gets overwritten by
current vnic outbound settings. Then, virNetDevBandwidthSet doesn't
change anything. We need to stop overwriting the outbound if users
don't want us to. Same applies for inbound.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If there should be some sort of separator it is better to use comment
with the filename, copyright, description, license information and
authors.
Found by:
git grep -nH '^$' | grep '\.[ch]:1:'
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This patch is not trying to fix every switch, just the ones I worked
with last time, because some of these were especially unreadable.
Covers enums virDomainGraphicsType and virDomainChrType (where
applicable).
Also sort its cases by their value.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit a1cbe4b5 added a check for spaces around assignments and this
patch extends it to checks for spaces around '=='. One exception is
virAssertCmpInt where comma after '==' is acceptable (since it is a
macro and '==' is its argument).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Coverity found an issue in lxc_driver and uml_driver that we don't
check the return value of register functions.
I've also updated all other places and unify the way we check the
return value.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Right now we are parsing the XML as though it's live, which for example
will choke on hardcoded XML like:
<seclabel type='dynamic' model='selinux' relabel='yes'/>
Erroring with:
$ sudo virsh domxml-to-native qemu-argv f
error: XML error: security label is missing
All drivers are fixed, but only qemu was tested.
We have to explicitly destroy TAP devices on FreeBSD because
they're not freed after being closed, otherwise we end up with
orphaned TAP devices after destroying a domain.
Many of the domain xml format functions (including all of the device
format functions) had hard-coded spaces, which made for incorrect
indentation when those functions were called in a different context
(for example, commit 2122cf39 added <interface> XML into the document
provided to a network hook script, and in this case it should have
been indented by 2 spaces, but was instead indented by 6 spaces).
To make it possible to insert a properly indented device anywhere into
an XML document, this patch removes hardcoded spaces from the
formatting functions, and calls virBufferAdjustIndent() at appropriate
places instead. (a regex search of domain_conf.c was done to assure
that all occurrences of hardcoded spaces were removed).
virDomainDiskSourceDefFormatInternal() is also called from
snapshot_conf.c, so two virBufferAdjustIndent() calls were temporarily
added around that call - those functions will have hardcoded spaces
removed in a separate patch.
This could cause some conflicts when backporting future changes to the
formatting functions to older branches, but fortunately the changes
are almost all trivial, so conflict resolution will be obvious.
When I played with virtlockd I was stunned by lacking
documentation. My frustration got bigger when I had to
read the patches to get the correct value to set in
qemu.conf.
Moreover, from pure libvirt-pride I'm changing commented
value from sanlock to lockd. We want to favor our own
implementation after all.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The kernel didn't support the unprivileged SGIO for SCSI generic
device finally, and since it's unknow whether the way to support
unprivileged SGIO for SCSI generic device will be similar as for
SCSI block device or not, even it's simliar (I.e. via sysfs, for
SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio,
for example), the file name might be different, So it's better not
guess what it should be like currently.
This patch removes the related code (mainly about the "shareable"
checking on the "sgio" setting, it's not supported at all, why
we leave checking code there? :-), and error out if "sgio" is
specified in the domain config.
Changes parameter from vm def to specific hostdevs info and name info, so that
it could be used more widely, e.g, could be used without full vm def info.
Some virHostdevXXXX methods included the string Hostdev again
as a suffix. Change the latter to Device instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Change any method names with Usb, Pci or Scsi to use
USB, PCI and SCSI since they are abbreviations.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Various methods in virnetdev.c and virhostdev.c were missing
const-ness for several char * parameters.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
For extracting hostdev codes from qemu_hostdev.c to common library, change qemu
specific COLD_BOOT handling to be a flag, and pass it to hostdev functions.
For extracting hostdev codes from qemu_hostdev.c to common library, change qemu
specific cfg->relaxedACS handling to be a flag, and pass it to hostdev
functions.
Same logic of preparing/reattaching hostdevs could be used in attach/detach
hotplug places, so reuse hostdev interfaces to avoid duplicate, also for later
extracting general code to common library.
Update parameters from vm->def to specific name, hostdevs, nhostdevs to keep
consistentcy with PreparePCIDevices and PrepareSCSIDevices. And, at the same
time, make it reusable in later patch.
When attaching to a QEMU process, the def->seclabels array is
going to be empty. The qemuProcessAttach method must thus
populate it with data for the security drivers.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
While investigating https://bugzilla.redhat.com/show_bug.cgi?id=1061827
I noticed that we pass user input unscathed for block-pull, but
always pass a canonical absolute name through for block-commit.
[Note that we probably _ought_ to validate that the user's request
for block-pull actually matches the backing chain, the way we already
do for block-commit - but that's a separate issue. Further note that
the ability to pass user input through unscathed allows backdoors
such as specifying a backing image that is a network URI such as
a gluster disk, instead of forcing things to the local file system;
which is an area still under active investigation on whether libvirt
needs to behave differently for network disks.]
Since qemu may write the name that the user passed in as the backing
file, a user may have a reason to want a relative file name passed
through to qemu, and always munging things to absolute prevents that.
Put another way, if you have the backing chain:
[A] <- [B(back=./A)] <- [C(back=./B)]
and commit B into A (virsh blockcommit $dom vda --base A --top B),
the metadata of C will have to be re-written. But should it be
rewritten as [C(back=./A)] or as [C(back=/path/to/A)]? Still up in
the air is whether qemu's decision should be based on whether B
and/or C had relative paths, or on whether the --base and/or
--top arguments to the command were relative paths; but if we always
pass a canonical name, we've prevented the spelling of the command
arguments from being part of the hueristics that qemu uses.
I also audited the code, and verified that we never call
qemuMonitorBlockCommit() with a NULL base, either before or after
the change to qemu_driver.c.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Preserve user's
spelling, since absolute vs. relative matters to qemu.
* src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): Base is never
null.
* src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The qemu_bridge_filter.c file had some helpers for calling
the ebtablesXXX functions todo bridge filtering. The only
thing these helpers did was to overwrite the original error
message from the ebtables code. For added fun, the callers
of these helpers overwrote the errors yet again. For even
more fun, one of the helpers called another helper and
overwrite its errors too.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Extracting capabilities from QEMU takes a notable amount of time
when all QEMU binaries are installed. Each system emulator
needs about 200-300ms multiplied by 26 binaries == ~5-8 seconds.
This change causes the QEMU driver to save an XML file containing
the content of the virQEMUCaps object instance in the cache
dir eg /var/cache/libvirt/qemu/capabilities/$SHA256(binarypath).xml
or $HOME/.cache/libvirt/qemu/cache/capabilities/$SHA256(binarypath).xml
We attempt to load this and only if it fails, do we fallback to
probing the QEMU binary. The ctime of the QEMU binary and libvirtd
are stored in the cached file and its data discarded if either
of them change.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Debian's package manager will preserve mtime timestamp on binaries
from the time they are built, rather than installed. So if a
user downgrades their QEMU dpkg, the libvirt capabilities
cache will not refresh. The fix is to use ctime instead of mtime
since it cannot be faked.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Avoid the freeing of an array of zero file descriptors in case
of error. Initialize the array to -1 using memset.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
This fixes a possible double free. In virNetworkAssignDef() if
virBitmapNew() fails, then virNetworkObjFree(network) is called.
However, with network->def pointing to actual @def. So if caller
frees @def again, ...
Moreover, this fixes one possible memory leak too. In
virInterfaceAssignDef() if appending to the list of interfaces
fails, we ought to call virInterfaceObjFree() instead of bare
VIR_FREE().
Although, in order to do that some array size variables needs
to be turned into size_t rather than int.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When domain is started with setting that cannot be done, i.e. those
that require cgroups, there is no error reported and it succeeds
without any message whatsoever.
When setting with API, virsh, an error is reported, but only due to
the fact that no cgroups are mounted (priv->cgroup == NULL).
Given the above it seems reasonable to reject such unsupported
settings.
This patch effectively changes the error message from:
$ virsh -c qemu:///session schedinfo dummy
Scheduler : Unknown
error: Requested operation is not valid: cgroup CPU controller is not mounted
to:
$ virsh -c qemu:///session schedinfo dummy
Scheduler : Unknown
error: Operation not supported: CPU tuning is not available in session mode
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1023366
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The qemuMonitorJSONBlockJob handles a few errors internally. If qemu
returns a different error we would report a rather unhelpful message:
$ virsh blockpull gluster-job vda --base /dev/null
error: internal error: Unexpected error
As the actual message from qemu contains a bit more info, let's use it
to report something a little more useful:
$ virsh blockpull gluster-job vda --base /dev/null
error: internal error: Unexpected error: (GenericError) 'Base '/dev/null' not found'
In qemuMonitorJSONExtractCPUInfo an error message hinted on missing
character device data which is wrong.
Also a comment states that only qemu-kvm tree includes the thread_id
field. This is no longer true.
https://bugzilla.redhat.com/show_bug.cgi?id=1071264
Reverting of external snapshots is not supported currently. The check
that is present doesn't properly check for all aspects that make a
snapshot external. Use virDomainSnapshotIsExternal() to do the check.
As I did previously in 4f588a1b46, libvirt needs to set virtio vectors.
Previously, we were advised to use vectors=N, where
N = 2 * (number of queues) + 1
However, just recently this advisory has changed on the Multiquue wiki
page [1] to:
N = 2 * (number of queues) + 2
1: http://www.linux-kvm.org/page/Multiqueue#Enable_MQ_feature
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Auditing all callers of virCommandRun and virCommandWait that
passed a non-NULL pointer for exit status turned up some
interesting observations. Many callers were merely passing
a pointer to avoid the overall command dying, but without
caring what the exit status was - but these callers would
be better off treating a child death by signal as an abnormal
exit. Other callers were actually acting on the status, but
not all of them remembered to filter by WIFEXITED and convert
with WEXITSTATUS; depending on the platform, this can result
in a status being reported as 256 times too big. And among
those that correctly parse the output, it gets rather verbose.
Finally, there were the callers that explicitly checked that
the status was 0, and gave their own message, but with fewer
details than what virCommand gives for free.
So the best idea is to move the complexity out of callers and
into virCommand - by default, we return the actual exit status
already cleaned through WEXITSTATUS and treat signals as a
failed command; but the few callers that care can ask for raw
status and act on it themselves.
* src/util/vircommand.h (virCommandRawStatus): New prototype.
* src/libvirt_private.syms (util/command.h): Export it.
* docs/internals/command.html.in: Document it.
* src/util/vircommand.c (virCommandRawStatus): New function.
(virCommandWait): Adjust semantics.
* tests/commandtest.c (test1): Test it.
* daemon/remote.c (remoteDispatchAuthPolkit): Adjust callers.
* src/access/viraccessdriverpolkit.c (virAccessDriverPolkitCheck):
Likewise.
* src/fdstream.c (virFDStreamCloseInt): Likewise.
* src/lxc/lxc_process.c (virLXCProcessStart): Likewise.
* src/qemu/qemu_command.c (qemuCreateInBridgePortWithHelper):
Likewise.
* src/xen/xen_driver.c (xenUnifiedXendProbe): Simplify.
* tests/reconnect.c (mymain): Likewise.
* tests/statstest.c (mymain): Likewise.
* src/bhyve/bhyve_process.c (virBhyveProcessStart)
(virBhyveProcessStop): Don't overwrite virCommand error.
* src/libvirt.c (virConnectAuthGainPolkit): Likewise.
* src/openvz/openvz_driver.c (openvzDomainGetBarrierLimit)
(openvzDomainSetBarrierLimit): Likewise.
* src/util/virebtables.c (virEbTablesOnceInit): Likewise.
* src/util/viriptables.c (virIpTablesOnceInit): Likewise.
* src/util/virnetdevveth.c (virNetDevVethCreate): Fix debug
message.
* src/qemu/qemu_capabilities.c (virQEMUCapsInitQMP): Add comment.
* src/storage/storage_backend_iscsi.c
(virStorageBackendISCSINodeUpdate): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
When probing QEMU capabilities fails for a binary generate a
log message with MESSAGE_ID==8ae2f3fb-2dbe-498e-8fbd-012d40afa361.
This can be directly queried from journald based on the UUID
instead of needing string grep. This lets tools like libguestfs'
bug reporting tool trivially do automated sanity tests on the
host they're running on.
$ journalctl MESSAGE_ID=8ae2f3fb-2dbe-498e-8fbd-012d40afa361
Feb 21 17:11:01 localhost.localdomain lt-libvirtd[9196]:
Failed to probe capabilities for /bin/qemu-system-alpha:
internal error: Child process (LC_ALL=C LD_LIBRARY_PATH=
/home/berrange/src/virt/libvirt/src/.libs PATH=/usr/lib64/
ccache:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:
/usr/bin:/root/bin HOME=/root USER=root LOGNAME=root
/bin/qemu-system-alpha -help) unexpected exit status 127:
/bin/qemu-system-alpha: error while loading shared libraries:
libglapi.so.0: cannot open shared object file: No such file
or directory
$ journalctl MESSAGE_ID=8ae2f3fb-2dbe-498e-8fbd-012d40afa361 --output=json
{ ...snip...
"LIBVIRT_SOURCE" : "file",
"PRIORITY" : "3",
"CODE_FILE" : "qemu/qemu_capabilities.c",
"CODE_LINE" : "2770",
"CODE_FUNC" : "virQEMUCapsLogProbeFailure",
"MESSAGE_ID" : "8ae2f3fb-2dbe-498e-8fbd-012d40afa361",
"LIBVIRT_QEMU_BINARY" : "/bin/qemu-system-xtensa",
"MESSAGE" : "Failed to probe capabilities for /bin/qemu-system-xtensa:
internal error: Child process (LC_ALL=C LD_LIBRARY_PATH=/home/berrange
/src/virt/libvirt/src/.libs PATH=/usr/lib64/ccache:/usr/local/sbin:
/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin HOME=/root
USER=root LOGNAME=root /bin/qemu-system-xtensa -help) unexpected
exit status 127: /bin/qemu-system-xtensa: error while loading shared
libraries: libglapi.so.0: cannot open shared object file: No such
file or directory\n" }
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
As of 0bd2ccdec an empty disk path for virDomainBlockStats (or the one
with Flags) is allowed meaning "get me overall summarized statistics".
However, running 'virsh domblkstat $dom' throws a misleading error:
# ./tools/virsh domblkstat dom
error: Failed to get block stats dom
error: invalid argument: invalid path:
while after this commit
# virsh domblkstat dom
error: Operation not supported: summary statistics are not supported yet
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1038363
If a domain has a different maximum for persistent and live maxmem
or max vcpus, then it is possible to hit cases where libvirt
refuses to adjust the current values or gets halfway through
the adjustment before failing. Better is to determine up front
if the change is possible for all requested flags.
Based on an idea by Geoff Franks.
* src/qemu/qemu_driver.c (qemuDomainSetMemoryFlags): Compute
correct maximum if both live and config are being set.
(qemuDomainSetVcpusFlags): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch changes network device type used by default from rtl8139
to virtio when architecture type is aarch64 and machine type is virt.
Qemu doesn't support any other machine types for aarch64 right now and
we can't make any other aarch64-specific tuning in this function yet.
Signed-off-by: Oleg Strikov <oleg.strikov@canonical.com>
There is no keyboard working on PPC64 and PS2 mouse is only for X86
when graphics are enabled.
Add a USB keyboard and USB mouse for PPC64 when graphics are enabled.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Format qemu command line for USB keyboard
and add test cases for it.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
There might be some use cases, where user wants to prepare the host or
its environment prior to starting a network and do some cleanup after
the network has been shut down. Consider all the functionality that
libvirt doesn't currently have as an example what a hook script can
possibly do.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, there's just one place where we care if hook script is
changing the domain XML: migration hook for incoming migration. In
all other places where a hook script is executed, we don't read the
XML back from the script.
Anyway, the hook script can alter domain XML and hence we should taint
it if the script did.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add support for gluster backed images as sources for snapshots in the
qemu driver. This will also simplify adding further network backed
volumes as sources for snapshot in case qemu will support them.
Use the new storage driver APIs to delete snapshot backing files in case
of failure instead of directly relying on "unlink". This will help us in
the future when we will be adding network based storage without local
representation in the host.
All the data for getting the actual type is present in the snapshot
config. There is no need to have this function private to the qemu
driver and it will be re-used later in other parts of libvirt
All the data for getting the actual type is present in the domain
config. There is no need to have this function private to the qemu
driver and it will be re-used later in other parts of libvirt
On some platforms like IBM PowerNV the NUMA node numbers can be
non-sequential. For eg. numactl --hardware o/p from such a machine looks
as given below
node distances:
node 0 1 16 17
0: 10 40 40 40
1: 40 10 40 40
16: 40 40 10 40
17: 40 40 40 10
The NUMA nodes are 0,1,16,17
Libvirt uses sequential index as NUMA node numbers and this can
result in crash or incorrect results.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Signed-off-by: Pradipta Kr. Banerjee <bpradip@in.ibm.com>
