mknod() is affected my the current umask, so we're not
guaranteed the newly-created device node will have the
right permissions.
Call chmod(), which is not affected by the current umask,
immediately afterwards to solve the issue.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1421036
To make sure bit 'b' fits into the bitmap, we need to allocate b+1
bits, since we number from 0.
Adjust the bitmap test to set a bit at a multiple of 16.
That way the test fails without this fix, because the VIR_REALLOC
call clears the newly added memory even if the original pointer
has not changed.
Move the range check introduced by commit 2650d5e into
virDomainUSBAddressFindPort. That way both virDomainUSBAddressRelease
and virDomainUSBAddressSetAddHub can benefit from it.
Reported-by: Michal Privoznik <mprivozn@redhat.com>
Due to a logic error, the autofilling of USB port when a bus is
specified:
<address type='usb' bus='0'/>
does not work for non-hub devices on domain startup.
Fix the logic in qemuDomainAssignUSBPortsIterator to also
assign ports for USB addresses that do not yet have one.
https://bugzilla.redhat.com/show_bug.cgi?id=1374128
We have to allocate first and if, and only if, it was successful we
can set the count. A segfault has occurred in
virNetServerServiceNewPostExecRestart() when VIR_ALLOC_N(svc->socks,
n) has failed, but svc->nsocsk = n was already set. Thus
virObejectUnref(svc) was called and therefore it was possible that
virNetServerServiceDispose was called => segmentation fault. For
safeness NULL pointer check were added in
virNetServerServiceDispose().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Recently e1000 NIC support was added to bhyve; implement that in
the bhyve driver:
- Add capability check by analyzing output of the 'bhyve -s 0,e1000'
command
- Modify bhyveBuildNetArgStr() to support e1000 and also pass
virConnectPtr so it could call bhyveDriverGetCaps() to check if this
NIC is supported
- Modify command parsing code to add support for e1000 and adjust tests
- Add net-e1000 test
If either the "if (STRPREFIX(mem_tokens[j], "max:"))" is never entered
or the "if (virStrToLong_ull(mem_tokens[j] + 4, &p, 10, maxmem) < 0)" break
is hit, control goes back to the outer loop processing 'cmd_tokens' and
it's possible that the 'mem_tokens' would be overwritten.
Found by Coverity
Right now, we use simple string comparison both on the source paths
(mount's output vs pool's source) and the target (mount's mnt_dir vs
pool's target). The problem are symlinks and mount indeed returns
symlinks in its output, e.g. /dev/mappper/lvm_symlink. The same goes for
the pool's source/target, so in order to successfully compare these two
replace plain string comparison with virFileComparePaths which will
resolve all symlinks and canonicalize the paths prior to comparison.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1417203
Signed-off-by: Erik Skultety <eskultet@redhat.com>
So rather than comparing 2 paths (strings) as they are, which can very
easily lead to unnecessary errors (e.g. in storage driver) that the paths
are not the same when in fact they'd be e.g. just symlinks to the same
location, we should put our best effort into resolving any symlinks and
canonicalizing the path and only then compare the 2 paths for equality.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When FS pool's source is already mounted on the target location instead
of just simply marking the pool as active, thus starting it we fail with
an error stating that the source is indeed already mounted on the target.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
On a system with 697 SCSI disks each configured with 8 paths the command
virsh nodedev-list fails with
error: Failed to list node devices
error: internal error: Too many node_devices '16816' for limit '16384'
Increasing the upper limit on lists of node devices from 16K to 64K.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Although currently this is documented in virsh man page
and virsh help, the expicit mention in the error message
is helful for tools using the API directly.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
==11846== 240 bytes in 1 blocks are definitely lost in loss record 81 of 107
==11846== at 0x4C2BC75: calloc (vg_replace_malloc.c:624)
==11846== by 0x18C74242: virAllocN (viralloc.c:191)
==11846== by 0x4A05E8: qemuMonitorCPUModelInfoCopy (qemu_monitor.c:3677)
==11846== by 0x446E3C: virQEMUCapsNewCopy (qemu_capabilities.c:2171)
==11846== by 0x437335: testQemuCapsCopy (qemucapabilitiestest.c:108)
==11846== by 0x437CD2: virTestRun (testutils.c:180)
==11846== by 0x437AD8: mymain (qemucapabilitiestest.c:176)
==11846== by 0x4397B6: virTestMain (testutils.c:992)
==11846== by 0x437B44: main (qemucapabilitiestest.c:188)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit removes the handcrafted code for
remoteDomainCreateWithFlags() and lets it auto generate.
A little bit of history repeating...
Commit 03d813bbcd removed the auto generation of
remoteDomainCreateWithFlags() because it was thought that the design
flaw in the remote protocol for virDomainCreate is also within the
remote protocol for virDomainCreateWithFlags. As the commit message of
ddaf15d7a3 mentions this is not the case therefore we
can auto generate the client part.
Even worse there was a typo in remoteDomainCreateWithFlags()
'remote_domain_create_with_flags_args ret;' but in fact it has to be
'remote_domain_create_with_flags_ret ret;'.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
After freeing the data structures we have to reset the counters to
zero. This fixes a segmentation fault when virNetDevIPInfoClear is
called twice (e.g. this is possible in virDomainNetDefParseXML() if
virDomainNetIPInfoParseXML(...) fails with ret < 0 (this leads to the
first call of 'virNetDevIPInfoClear(&def->guestIP)') and the resulting
call of virDomainNetDefFree(def) in the error path of
virDomainNetDefParseXML() (this leads to the second call of
virNetDevIPInfoClear(&def->guestIP), and finally to the segmentation
fault).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
If virDomainChrSourceDefNew(xmlopt) fails, it will lead to free()ing
the uninitialized pointer bus. The fix for this is to initialize bus
with NULL.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Check if virQEMUCapsNewCopy(...) has failed, thus a segmentation fault
in virQEMUCapsFilterByMachineType(...) will be avoided.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Using libvirt to do live migration over RDMA via IPv6 address failed.
For example:
rhel73_host1_guest1 qemu+ssh://[deba::2222]/system --verbose
root@deba::2222's password:
error: internal error: unable to execute QEMU command 'migrate': RDMA
ERROR: could not rdma_getaddrinfo address deba
As we can see, the IPv6 address used by rdma_getaddrinfo() has only
"deba" part because we didn't properly enclose the IPv6 address in []
and passed rdma:deba::2222:49152 as the migration URI in
qemuMonitorMigrateToHost.
Signed-off-by: David Dai <zdai@linux.vnet.ibm.com>
When the libxl driver is initialized, it creates a virDomainDef
object for dom0 and adds it to the list of domains. Total memory
for dom0 was being set from the max_memkb field of libxl_dominfo
struct retrieved from libxl, but this field can be set to
LIBXL_MEMKB_DEFAULT (~0ULL) if dom0 maximum memory has not been
explicitly set by the user.
This patch adds some simple parsing of the Xen commandline,
looking for a dom0_mem parameter that also specifies a 'max' value.
If not specified, dom0 maximum memory is effectively all physical
host memory.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The libxl driver reports different values of maximum memory depending
on state of a domain. If inactive, maximum memory value is reported
correctly. When active, maximum memory is derived from max_pages value
returned by the XEN_SYSCTL_getdomaininfolist sysctl operation. But
max_pages can be changed by toolstacks and does not necessarily
represent the maximum memory a domain can use during its active
lifetime.
A better location for determining a domain's maximum memory is the
/local/domain/<id>/memory/static-max node in xenstore. This value
is set from the libxl_domain_build_info.max_memkb field when creating
the domain. Currently it cannot be changed nor can its value be
exceeded by a balloon operation. From libvirt's perspective, always
reporting maximum memory with virDomainDefGetMemoryTotal() will produce
the same results as reading the static-max node in xenstore.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
When a user does not explicitly set a <driver> in the disk config,
libvirt defers selection of a default to libxl. This approach works
fine when starting a domain with such configuration or attaching a
disk to a running domain. But when detaching such a disk, libxl
will fail with "unrecognized disk backend type: 0". libxl makes no
attempt to recalculate a default backend (driver) on detach and
simply fails when uninitialized.
This patch updates the libvirt disk config with the backend selected
by libxl when starting a domain or attaching a disk to a running
domain. Another benefit of this approach is that the live XML is
also updated with the backend driver selected by libxl.
When starting a domian, a libxl_domain_config object is created from
virDomainDef. Any virDomainDiskDef devices with a format of
VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
corresponding libxl_disk_device, but the virDomainDiskDef format is
never updated to reflect the change.
A better place to set a default format for disk devices is the
device post-parse callback, ensuring the virDomainDiskDef object
reflects the default format.
This patchs allows to set the timeout value used for all
openvswitch calls. The default timeout value remains as
before at 5 seconds.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Provide the ability to specify a default timeout value for
successful completion of openvswitch calls in the libvirtd
configuration file.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch add support for file memory backing on numa topology.
The specified access mode in memoryBacking can be overriden
by specifying token memAccess in numa cell.
This part introduces new xml elements for file based
memorybacking support and their parsing.
(It allows vhost-user to be used without hugepages.)
New xml elements:
<memoryBacking>
<source type="file|anonymous"/>
<access mode="shared|private"/>
<allocation mode="immediate|ondemand"/>
</memoryBacking>
Add new parameter memory_backing_dir where files will be stored when memoryBacking
source is selected as file.
Value is stored inside char* memoryBackingDir
Rename to avoid duplicate code. Because virDomainMemoryAccess will be
used in memorybacking for setting default behaviour.
NOTE: The enum cannot be moved to qemu/domain_conf because of headers
dependency
Strings associated with virDomainHyperv values in domain_conf.c are used to
construct HyperV CPU features names to be compared with names defined in
cpu_x86_data.h and the names for HyperV "spinlocks" feature don't match.
This leads to a misleading warning:
"host doesn't support hyperv 'spinlocks' feature" even when it's supported.
Let's fix it and rename along with it VIR_CPU_x86_KVM_HV_SPINLOCK to
VIR_CPU_x86_KVM_HV_SPINLOCKS.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
virCPUDefStealModel is called with keepVendor == true which means the
cpu structure will keep its original vendor/vendor_id values. Thus it
makes no sense to copy them to the translated definition as they won't
be used there anyway. Except that the translated->vendor pointer might
get lost in x86Decode.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When we happen to lose a domain but still get a performance event
for it, we should also free the event handle.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Just like we need wrappers over other virSecurityManager APIs, we
need one for virSecurityManagerSetImageLabel and
virSecurityManagerRestoreImageLabel. Otherwise we might end up
relabelling device in wrong namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The typical pattern when calling libxl functions that populate a
structure is
libxl_foo foo;
libxl_foo_init(&foo);
libxl_get_foo(ctx, &foo);
...
libxl_foo_dispose(&foo);
Fix several instances of libxl_physinfo missing the init and
dispose calls.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
libxlGetAutoballoonConf is supposed to honor user-specified
autoballoon setting in libxl.conf. As written, the user-specified
setting could be overwritten by the subsequent logic to check
dom0_mem parameter. If user-specified setting is present and
correct, accept it. Only fallback to checking Xen dom0_mem
command line parameter if user-specfied setting is not present.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
If no graphics element is in XML xenFormatXLSpice will access
graphics without checking it has one in the first place, leading to a
segmentation fault.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Firstly, instead of checking for next->path the
virStorageSourceIsEmpty() function should be used which also
takes disk type into account.
Secondly, not every disk source passed has the correct type set
(due to our laziness). Therefore, instead of checking for
virStorageSourceIsBlockLocal() and also S_ISBLK() the former can
be refined to just virStorageSourceIsLocalStorage().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Again, one missed bit. This time without this commit there is no
/dev entry in the namespace of the qemu process when doing disk
snapshots or block-copy.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
These functions do not need to see the whole virDomainDiskDef.
Moreover, they are going to be called from places where we don't
have access to the full disk definition. Sticking with
virStorageSource is more than enough.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There is no need for this. None of the namespace helpers uses it.
Historically it was used when calling secdriver APIs, but we
don't to that anymore.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Again, one missed bit. This time without this commit there is no
/dev entry in the namespace of the qemu process when attaching
vhost SCSI device.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since we have qemuSecurity wrappers over
virSecurityManagerSetHostdevLabel and
virSecurityManagerRestoreHostdevLabel we ought to use them
instead of calling secdriver APIs directly. Without those
wrappers the labelling won't be done in the correct namespace
and thus won't apply to the nodes seen by qemu itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
libvirt was able to set the host_mtu option when an MTU was explicitly
given in the interface config (with <mtu size='n'/>), set the MTU of a
libvirt network in the network config (with the same named
subelement), and would automatically set the MTU of any tap device to
the MTU of the network.
This patch ties that all together (for networks based on tap devices
and either Linux host bridges or OVS bridges) by learning the MTU of
the network (i.e. the bridge) during qemuInterfaceBridgeConnect(), and
returning that value so that it can then be passed to
qemuBuildNicDevStr(); qemuBuildNicDevStr() then sets host_mtu in the
interface's commandline options.
The result is that a higher MTU for all guests connecting to a
particular network will be plumbed top to bottom by simply changing
the MTU of the network (in libvirt's config for libvirt-managed
networks, or directly on the bridge device for simple host bridges or
OVS bridges managed outside of libvirt).
One question I have about this - it occurred to me that in the case of
migrating a guest from a host with an older libvirt to one with a
newer libvirt, the guest may have *not* had the host_mtu option on the
older machine, but *will* have it on the newer machine. I'm curious if
this could lead to incompatibilities between source and destination (I
guess it all depends on whether or not the setting of host_mtu has a
practical effect on a guest that is already running - Maxime?)
Likewise, we could run into problems when migrating from a newer
libvirt to older libvirt - The guest would have been told of the
higher MTU on the newer libvirt, then migrated to a host that didn't
understand <mtu size='blah'/>. (If this really is a problem, it would
be a problem with or without the current patch).
Example:
<network>
...
<mtu size='9000'/>
...
If mtu is unset, it's assumed that we want the default for whatever is
the underlying transport (usually this is 1500).
This setting isn't yet wired in, so it will have no effect.
This partially resolves: https://bugzilla.redhat.com/1224348
virNetDevTapCreateInBridgePort() has always set the new tap device to
the current MTU of the bridge it's being attached to. There is one
case where we will want to set the new tap device to a different
(usually larger) MTU - if that's done with the very first device added
to the bridge, the bridge's MTU will be set to the device's MTU. This
patch allows for that possibility by adding "int mtu" to the arg list
for virNetDevTapCreateInBridgePort(), but all callers are sending -1,
so it doesn't yet have any effect.
Since the requested MTU isn't necessarily what is used in the end (for
example, if there is no MTU requested, the tap device will be set to
the current MTU of the bridge), and the hypervisor may want to know
the actual MTU used, we also return the actual MTU to the caller (if
actualMTU is non-NULL).
In order for memory locking to work, the hard limit on memory
locking (and usage) has to be set appropriately by the user.
The documentation mentions the requirement already: with this
patch, it's going to be enforced by runtime checks as well,
by forbidding a non-compliant guest from being defined as well
as edited and started.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1316774
Like it usually happens, I fixed one thing and broke another:
in 803966c76 address allocation was fixed for SATA disks, but
broke that for virtio disks, because it dropped disk address
assignment completely. It's not needed for SATA disks anymore,
but still needed for the virtio ones.
Bring that back and add a couple of tests to make sure it won't
happen again.
Similarly to one of the previous commits, we need to deal
properly with symlinks in hotplug case too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Imagine you have a disk with the following source set up:
/dev/disk/by-uuid/$uuid (symlink to) -> /dev/sda
After cbc45525cb the transitive end of the symlink chain is
created (/dev/sda), but we need to create any item in chain too.
Others might rely on that.
In this case, /dev/disk/by-uuid/$uuid comes from domain XML thus
it is this path that secdriver tries to relabel. Not the resolved
one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After previous commit this has become redundant step.
Also setting up devices in namespace and setting their label
later on are two different steps and should be not done at once.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The idea is to move all the seclabel setting to security driver.
Having the relabel code spread all over the place looks very
messy.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Because of the nature of security driver transactions, it is
impossible to use them properly. The thing is, transactions enter
the domain namespace and commit all the seclabel changes.
However, in RestoreAllLabel() this is impossible - the qemu
process, the only process running in the namespace, is gone. And
thus is the namespace. Therefore we shouldn't use the transactions
as there is no namespace to enter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The current ordering is as follows:
1) set label
2) create the device in namespace
3) allow device in the cgroup
While this might work for now, it will definitely not work if the
security driver would use transactions as in that case there
would be no device to relabel in the domain namespace as the
device is created in the second step.
Swap steps 1) and 2) to allow security driver to use more
transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We will need to traverse the symlinks one step at the time.
Therefore we need to see where a symlink is pointing to.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The comment to the function states that the errors from the child
process are reported. Well, the error buffer is filled with
possible error messages. But then it is thrown away. Among with
important error message from the child process.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
==11260== 1,006 bytes in 1 blocks are definitely lost in loss record 106 of 111
==11260== at 0x4C2AE5F: malloc (vg_replace_malloc.c:297)
==11260== by 0x4C2BDFF: realloc (vg_replace_malloc.c:693)
==11260== by 0x4EA430B: virReallocN (viralloc.c:245)
==11260== by 0x4EA7C52: virBufferGrow (virbuffer.c:130)
==11260== by 0x4EA7D28: virBufferAdd (virbuffer.c:165)
==11260== by 0x4EA8E10: virBufferStrcat (virbuffer.c:718)
==11260== by 0x42D263: xenFormatXLDiskSrcNet (xen_xl.c:960)
==11260== by 0x42D4EB: xenFormatXLDiskSrc (xen_xl.c:1015)
==11260== by 0x42D870: xenFormatXLDisk (xen_xl.c:1101)
==11260== by 0x42DA89: xenFormatXLDomainDisks (xen_xl.c:1148)
==11260== by 0x42EAF8: xenFormatXL (xen_xl.c:1558)
==11260== by 0x40E85F: testCompareParseXML (xlconfigtest.c:105)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Originally/discovered proposed by "Wang King <king.wang@huawei.com>"
When the virCloseCallbacksSet is first called, it increments the refcnt
on the domain object to ensure it doesn't get deleted before the callback
is called. The refcnt would be decremented in virCloseCallbacksUnset once
the entry is removed from the closeCallbacks has table.
When (mostly) normal shutdown occurs, the qemuProcessStop will end up
calling qemuProcessAutoDestroyRemove and will remove the callback from
the list and hash table normally and decrement the refcnt.
However, when qemuConnectClose calls virCloseCallbacksRun, it will scan
the (locked) closeCallbacks list for matching domain and callback function.
If an entry is found, it will be removed from the closeCallbacks list and
placed into a lookaside list to be processed when the closeCallbacks lock
is dropped. The callback function (e.g. qemuProcessAutoDestroy) is called
and will run qemuProcessStop. That code will fail to find the callback
in the list when qemuProcessAutoDestroyRemove is called and thus not decrement
the domain refcnt. Instead since the entry isn't found the code will just
return (mostly) harmlessly.
This patch will resolve the issue by taking another ref during the
search UUID process during virCloseCallackRun, decrementing the refcnt
taken by virCloseCallbacksSet, calling the callback routine and returning
overwriting the vm (since it could return NULL). Finally, it will call the
virDomainObjEndAPI to lower the refcnt and remove the lock taken during
the search UUID processing. This may cause the vm to be destroyed.
After deploying virtlogd by default we identified a number of
mistakes in the systemd unit file. virtlockd's relationship
to libvirtd is the same as virtlogd, so we must apply the
same unit file fixes to virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
xen.git commit 57f8b13c changed several of the libxl memory
get/set functions to take 64 bit parameters. The libvirt
libxl driver still uses uint32_t variables for these various
parameters, which is particularly problematic for the
libxl_set_memory_target() function.
When dom0 autoballooning is enabled, libvirt (like xl) determines
the memory needed to start a domain and the memory available. If
memory available is less than memory needed, dom0 is ballooned
down by passing a negative value to libxl_set_memory_target()
'target_memkb' parameter. Prior to xen.git commit 57f8b13c,
'target_memkb' was an int32_t. Subtracting a larger uint32 from
a smaller uint32 and assigning it to int32 resulted in a negative
number. After commit 57f8b13c, the same subtraction is widened
to a int64, resulting in a large positive number. The simple
fix taken by this patch is to assign the difference of the
uint32 values to a temporary int32 variable, which is then
passed to 'target_memkb' parameter of libxl_set_memory_target().
Note that it is undesirable to change libvirt to use 64 bit
variables since it requires setting LIBXL_API_VERSION to 0x040800.
Currently libvirt supports LIBXL_API_VERSION >= 0x040400,
essentially Xen >= 4.4.
Now that we have a function for properly assigning the blockdeviotune
info, let's use it instead of dropping the group name on every
assignment. Otherwise it will not work with both --live and --config
options.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This is necessary to be able to get statistics for venet0 or
"host-routed" adapter, which has -1 index and thus, its statistics
is shown as "net.nic4294967295".
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Commit 815d98a started auto-adding one hub if there are more USB devices
than available USB ports.
This was a strange choice, since there might be even more devices.
Before USB address allocation was implemented in libvirt, QEMU
automatically added a new USB hub if the old one was full.
Adjust the logic to try adding as many hubs as will be needed
to plug in all the specified devices.
https://bugzilla.redhat.com/show_bug.cgi?id=1410188
As bhyve for a long time didn't have a notion of the explicit SATA
controller and created a controller for each drive, the bhyve driver
in libvirt acted in a similar way and didn't care about the SATA
controllers and assigned PCI addresses to drives directly, as
the generated command will look like this anyway:
2:0,ahci-hd,somedisk.img
This no longer makes sense because:
1. After commit c07d1c1c4f it's not possible to assign
PCI addresses to disks
2. Bhyve now supports multiple disk drives for a controller,
so it's going away from 1:1 controller:disk mapping, so
the controller object starts to make more sense now
So, this patch does the following:
- Assign PCI address to SATA controllers (previously we didn't do this)
- Assign disk addresses instead of PCI addresses for disks. Now, when
building a bhyve command, we take PCI address not from the disk
itself but from its controller
- Assign addresses at XML parsing time using the
assignAddressesCallback. This is done mainly for being able to
verify address allocation via xml2xml tests
- Adjust existing bhyvexml2{xml,argv} tests to chase the new
address allocation
This patch is largely based on work of Fabian Freyer.
Add virBhyveDriverCreateXMLConf, a simple wrapper around
virDomainXMLOptionNew that makes it easier to pass bhyveConnPtr
as a private data for parser. It will be used later for device
address allocation at parsing time.
Update consumers to use it instead of direct calls to
virDomainXMLOptionNew.
As we now have proper callbacks connected for the tests, update
test files accordingly to include the automatically generated
PCI root controller.
Introduce a BHYVE_CAP_AHCI32SLOT capability that shows
if 32 devices per SATA controller are supported, and
a bhyveProbeCapsAHCI32Slot function that probes it.
Because this is invalid xml for containers. This patch almost
reverts 7eda8369, but still skips converting vz sdk bootorder
for containers to libvirt bootorder because we use boot order
in containers for quite different purpurse.
==12618== 110 bytes in 10 blocks are definitely lost in loss record 269 of 295
==12618== at 0x4C2AE5F: malloc (vg_replace_malloc.c:297)
==12618== by 0x1CFC6DD7: vasprintf (vasprintf.c:73)
==12618== by 0x1912B2FC: virVasprintfInternal (virstring.c:551)
==12618== by 0x1912B411: virAsprintfInternal (virstring.c:572)
==12618== by 0x50B1FF: qemuAliasChardevFromDevAlias (qemu_alias.c:638)
==12618== by 0x518CCE: qemuBuildChrChardevStr (qemu_command.c:4973)
==12618== by 0x522DA0: qemuBuildShmemBackendChrStr (qemu_command.c:8674)
==12618== by 0x523209: qemuBuildShmemCommandLine (qemu_command.c:8789)
==12618== by 0x526135: qemuBuildCommandLine (qemu_command.c:9843)
==12618== by 0x48B4BA: qemuProcessCreatePretendCmd (qemu_process.c:5897)
==12618== by 0x4378C9: testCompareXMLToArgv (qemuxml2argvtest.c:498)
==12618== by 0x44D5A6: virTestRun (testutils.c:180)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For example when both total_bytes_sec and total_bytes_sec_max are set,
but the former gets cleaned due to new call setting, let's say,
read_bytes_sec, we end up with this weird message for the command:
$ virsh blkdeviotune fedora vda --read-bytes-sec 3000
error: Unable to change block I/O throttle
error: unsupported configuration: value 'total_bytes_sec_max' cannot be set if 'total_bytes_sec' is not set
So let's make it more descriptive. This is how it looks after the change:
$ virsh blkdeviotune fedora vda --read-bytes-sec 3000
error: Unable to change block I/O throttle
error: unsupported configuration: cannot reset 'total_bytes_sec' when 'total_bytes_sec_max' is set
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1344897
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
We were setting it based on whether it was supported and that lead to
setting it to NULL, which our JSON code caught. However it ended up
producing the following results:
$ virsh blkdeviotune fedora vda --total-bytes-sec-max 2000
error: Unable to change block I/O throttle
error: internal error: argument key 'group' must not have null value
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The issue is that if this graphics definition is provided:
<graphics type='vnc' port='0'/>
it's parsed as:
<graphics type='vnc' autoport='no'>
<listen type='address'/>
</graphics>
but if the resulting XML is parsed again the output is:
<graphics type='vnc' port='-1' autoport='yes'>
<listen type='address'/>
</graphics>
and this should not happen. The XML have to always remain the same
after it was already parsed by libvirt.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1383039
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Currently, on every --enable perf_event command,
a new event->fd is created and counting of perf
event counter starts from zero and previous
event->fd is lost. This patch prevents this
behaviour.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
Commit id '5f07c3c07' broke the freebsd build in the libvirt CI test
environment because the UMOUNT was not defined unless WITH_STORAGE_FS
is defined.
So remove the virStorageBackendUmountLocal from storage_util.c,h and
restore the code back in the storage_backend_fs.c and _vstorage.c
modules.
Added create/define/etc pool operations for vstorage backend.
Used the common/local pool API's from storage_util for operations
that are not specific to vstorage. In particular Refresh and Delete
Pool operations as well as all the Volume operations.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
Added general definitions for vstorage pool backend including
the build options to add --with-storage-vstorage checking.
In order to use vstorage as a backend for a storage pool
vstorage tools (vstorage and vstorage-mount) need to be installed.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
Move all the volume functions to storage_util to create local/common helpers
using the same naming syntax as the existing upload, download, and wipe
virStorageBackend*Local API's.
In the process of doing so, found more API's that can now become local
to storage_util. In order to distinguish between local/external - I
changed the names of the now local only ones from "virStorageBackend..."
to just "storageBackend..."
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move some pool functions to storage_util to create local/common helpers
using the same naming syntax as the existing upload, download, and wipe
virStorageBackend*Local API's.
In the process of doing so, found a few API's that can now become local
to storage_util. In order to distinguish between local/external - I
changed the names of the now local only ones from "virStorageBackend..."
to just "storageBackend..."
Signed-off-by: John Ferlan <jferlan@redhat.com>
Just moving code around with minor adjustment to have the Stop
code combine with the Unmount code since all the Stop code did
was call the Unmount code.
Not only we should set the MTU on the host end of the device but
also let qemu know what MTU did we set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So far we allow to set MTU for libvirt networks. However, not all
domain interfaces have to be plugged into a libvirt network and
even if they are, they might want to have a different MTU (e.g.
for testing purposes).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We use @ret to hold the actual return value of the function we
are currently in. To hold a return value of a function called we
use different variables: @rv, @rc, etc. Honour this naming
scheme in virDomainNetDefParseXML too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The current logic around configuring timers in libxl based on
virDomainDef object is a bit brain dead. Unsupported timers are
silently ignored and tsc is only recognized if it is the first
timer specified.
Change the logic to reject unsupported timers and honor the tsc
timer regardless of its order when multiple timers are specified.
It is destructive to attempt reset on a pci- or cardbus-bridge, the
host can crash. The bridges won't contain any guest data and neither
they can be passed through using vfio/stub. So, no point in allowing a
reset on them.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Non-endpoint devices like pci-bridges cannot be assigned to guests.
Prevent such attempts.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Avoid return with the closeCallbacks locked when get callbacks list for connect fail.
Signed-off-by: Wang King <king.wang@huawei.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
We lacked of timestamp in tainting of guests log,
which bring troubles for finding guest issues:
such as whether a guest powerdown caused by qemu-monitor-command
or others issues inside guests.
If we had timestamp in tainting of guests log,
it would be helpful when checking guest's /var/log/messages.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
==24748== 12 bytes in 2 blocks are definitely lost in loss record 25 of 84
==24748== at 0x4C2BF80: malloc (vg_replace_malloc.c:296)
==24748== by 0x1A1E1E78: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4)
==24748== by 0x18D0495F: virXMLPropString (virxml.c:506)
==24748== by 0x18D1FB3E: virDomainHostdevSubsysSCSIVHostDefParseXML (domain_conf.c:6280)
==24748== by 0x18D20350: virDomainHostdevDefParseXMLSubsys (domain_conf.c:6450)
==24748== by 0x18D34E7D: virDomainHostdevDefParseXML (domain_conf.c:13218)
==24748== by 0x18D42598: virDomainDefParseXML (domain_conf.c:17745)
==24748== by 0x18D440A9: virDomainDefParseNode (domain_conf.c:18236)
==24748== by 0x18D43EFA: virDomainDefParse (domain_conf.c:18180)
==24748== by 0x18D43FA0: virDomainDefParseFile (domain_conf.c:18206)
==24748== by 0x44EDA1: testCompareDomXML2XMLFiles (testutils.c:1140)
==24748== by 0x4365F8: testXML2XMLActive (qemuxml2xmltest.c:59)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Based on work of Mehdi Abaakouk <sileht@sileht.net>.
When parsing vhost-user interface XML and no ifname is found we
can try to fill it in in post parse callback. The way this works
is we try to make up interface name from given socket path and
then ask openvswitch whether it knows the interface.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The event needs to be emitted after the last monitor call, so that it's
not possible to find the device in the XML accidentally while the vm
object is unlocked.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1414393
When generating a domain XML from native command (i.e. via
the connectDomainXMLFromNative call), we should use
interface type 'bridge' rather than 'ethernet' because we only
support bridges at this point.
As we don't have bridge name explicitly specified on the command line,
just use 'virbr0' as a default.
Previous commit tried to change configure logic such that the
GLUSTER_CLI parameter would always be set:
commit 9e97c8c0f0
Author: Peter Krempa <pkrempa@redhat.com>
Date: Mon Jan 9 15:56:12 2017 +0100
storage: gluster: Remove build-time dependency on the 'gluster' cli tool
This missed the fact that the AC_PATH_PROG call was itself inside an 'if'
conditional that would not be called in with_storage_gluster was false. As
a result, GLUSTER_CLI was still conditionally defined.
Just kill the GLUSTER_CLI parameter and AC_PATH_PROG call entirely and pass a
bare "gluster" string to virFindFileInPath instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Previously when QEMU failed "drive_add" due to an error opening
a file it would report
"could not open disk image"
These days though, QEMU reports
"Could not open '/tmp/virtd-test_e3hnhh5/disk1.qcow2': Permission denied"
which we were not detecting as an error condition.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The iSCSI backend driver was using stuff from the SCSI driver without
making sure that it's compiled in. Move the common code into the
storage_util.c since it does not contain any specific code.
The file backend code was mistakenly put into #if WITH_STORAGE_FS. This
is not necessary since all the backends just access files on disk, and
thus the code for WITH_STORAGE_DIR is sufficient to compile everything.
The file became a garbage dump for all kinds of utility functions over
time. Move them to a separate file so that the files can become a clean
interface for the storage backends.
fabric_name is one of many fc_host attributes in Linux that is optional
and left to the low-level driver to decide if it is implemented.
The zfcp device driver does not provide a fabric name for an fcp host.
This patch removes the requirement for a fabric name by making it optional.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
File open errors are prevented by a file exists check before
virFileReadAll is called since all callers of the virReadFCHost
method handle errors themselves based on the NULL return anyway.
Also included is a minor spelling correction in a comment.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1346566
If libvirt_parthelper is erroneously told to append the partition
separator 'p' onto the generated output for a disk pool using device
mapper that has 'user_friendly_names' set to true, then the error
recovery path will fail to find volume resulting in the pool being
in an unusable state.
So, augment the documentation to provide the better hint that the
part_separator='yes' should be set when user_friendly_names are not
being used. Additionally, once we're in the error path where the
returned name doesn't match the expected partition name try to see
if the reason is because the 'p' was erroneosly added. If so alter
the about to be removed vol->target.path so that the DiskDeleteVol
code can find the partition that was created and remove it.
If the voldef type is VIR_STORAGE_VOL_BLOCK, then as long as the
format is known, let's allow the probe to happen - gets a truer value
and the same probe/update would be allowed for the same volume defined
in a domain.
For volume processing in virStorageBackendUpdateVolTargetInfo to get
the capacity commit id 'a760ba3a7' added the ability to probe a volume
that didn't list a target format. Unfortunately, the code used the
virStorageSource (e.g. target->type - virStorageType) rather than
virStorageVolDef (e.g. vol->type - virStorageVolType) in order to
make the comparison. As it turns out target->type for a volume is
not filled in at all for a voldef as the code relies on vol->type.
Ironically the result is that only VIR_STORAGE_VOL_BLOCK's would get
their capacity updated.
This patch will adjust the code to check the "vol->type" field instead
as an argument. This way for a voldef, the correct comparison is made.
Additionally for a backingStore, the 'type' field is never filled in;
however, since we know that the provided path is a location at which
the backing store can be accessed on the local filesystem thus just
pass VIR_STORAGE_VOL_FILE in order to satisfy the adjusted voltype
check. Whether it's a FILE or a BLOCK only matters if we're trying to
get more data based on the target->format.
The tool is used for pool discovery. Since we call an external binary we
don't really need to compile out the code that uses it. We can check
whether it exists at runtime.
In commit 4090e15399 we went back from reporting no errors if no storage
pools were found on a given host to reporting a bad error. And only in
cases when gluster was not installed.
Report a less bad error in case there are no volumes. Also report the
error when gluster is installed but no volumes were found, since
virStorageBackendFindGlusterPoolSources would return success in that
case.
Extract the call to qemuDomainSelectHotplugVcpuEntities outside of
qemuDomainSetVcpusLive and decide whether to hotplug or unplug the
entities specified by the cpumap using a boolean flag.
This will allow to use qemuDomainSetVcpusLive in cases where we prepare
the list of vcpus to enable or disable by other means.
In cases where CPU hotplug is supported by qemu force the monitor to
reject invalid or broken responses to 'query-cpus'. It's expected that
the command returns usable data in such case.
The problem is in the way how the list item is created prior to
appending it to the transaction list - the @path argument is just a
shallow copy instead of deep copy of the hostdev device's path.
Unfortunately, the hostdev devices from which the @path is extracted, in
order to add them into the transaction list, are only temporary and
freed before the buildup of the qemu namespace, thus making the @path
attribute in the transaction list NULL, causing 'permission denied' or
'double free' or 'unknown cause' errors.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1413773
Signed-off-by: Erik Skultety <eskultet@redhat.com>
The problem is in the way how the list item is created prior to
appending it to the transaction list - the @path attribute is just a
shallow copy instead of deep copy of the hostdev device's path.
Unfortunately, the hostdev devices from which the @path is extracted, in
order to add them into the transaction list, are only temporary and
freed before the buildup of the qemu namespace, thus making the @path
attribute in the transaction list NULL, causing 'permission denied' or
'double free' or 'unknown cause' errors.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1413773
Signed-off-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1413922
While all the code that deals with qemu namespaces correctly
detects whether we are running as root (and turn into NO-OP for
qemu:///session) the actual unshare() call is not guarded with
such check. Therefore any attempt to start a domain under
qemu:///session shall fail as unshare() is reserved for root.
The fix consists of moving unshare() call (for which we have a
wrapper called virProcessSetupPrivateMountNS) into
qemuDomainBuildNamespace() where the proper check is performed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
For case VIR_STORAGE_BLKID_PROBE_DIFFERENT, clean up the message to
avoid using the virsh like --overwrite syntax. Additionally provide
a different error message when not writing the label to avoid confusion.
Rather than special casing the VIR_STORAGE_BLKID_PROBE_UNKNOWN after
calling virStorageBackendBLKIDFindPart, just allow the switch statement
handle setting ret = -2.
If neither BLKID or PARTED is available and we're not writing, then
just return 0 which allows the underlying storage pool to generate
a failure. If both are unavailable and we're writing, then generate
a more generic error message.
When running on s390 with a kernel that does not support cpu model checking and
with a Qemu new enough to support query-cpu-model-expansion, the gathering of qemu
capabilities will fail. Qemu responds to the query-cpu-model-expansion qmp
command with an error because the needed kernel ioct does not exist. When this
happens a guest cannot even be defined due to missing qemu capabilities data.
This patch fixes the problem by silently ignoring generic errors stemming from
calls to query-cpu-model-expansion.
Reported-by: Farhan Ali <alifm@linux.vnet.ibm.com>
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
When creating new /dev/* for qemu, we do chown() and copy ACLs to
create the exact copy from the original /dev. I though that
copying SELinux labels is not necessary as SELinux will chose the
sane defaults. Surprisingly, it does not leaving namespace with
the following labels:
crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 random
crw-------. root root system_u:object_r:tmpfs_t:s0 rtc0
drwxrwxrwt. root root system_u:object_r:tmpfs_t:s0 shm
crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 urandom
As a result, domain is unable to start:
error: internal error: process exited while connecting to monitor: Error in GnuTLS initialization: Failed to acquire random data.
qemu-kvm: cannot initialize crypto: Unable to initialize GNUTLS library: Failed to acquire random data.
The solution is to copy the SELinux labels as well.
Reported-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The check is pointless since LVM is capable to detect it's own members
and the check is flawed as it would fail if neither libblkid nor parted
is installed.
We don't really need to babysit LVM in this way.
This reverts commit cb38b6cbc7.
