pSeries guests will soon be allowed to have multiple
PHBs (pci-root controllers), which of course means that
all but one of them will have a non-zero index; hence,
we'll need to relax the current check.
However, right now the check is performed in the conf
module, which is generic rather than tied to the QEMU
driver, and where we don't have information such as the
guest machine type available.
To make this change of behavior possible down the line,
we need to move the check from the XML parser to the
drivers. Luckily, only QEMU and bhyve are using PCI
controllers, so this doesn't result in much duplication.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Add support for vgaconf driver configuration. In domain xml it looks like
this:
<video>
<driver vgaconf='io|on|off'>
<model .../>
</video>
It was added with bhyve gop video in mind to allow users control how the
video device is exposed to the guest, specifically, how VGA I/O is
handled.
One can refer to the bhyve manual page to get more detailed description
of the possible VGA configuration options:
https://www.freebsd.org/cgi/man.cgi?query=bhyve&manpath=FreeBSD+12-current
The relevant part could be found using the 'vgaconf' keyword.
Also, add some tests for this new feature.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If a remote call fails during event registration (more than likely from
a network failure or remote libvirtd restart timed just right), then when
calling the virObjectEventStateDeregisterID we don't want to call the
registered @freecb function because that breaks our contract that we
would only call it after succesfully returning. If the @freecb routine
were called, it could result in a double free from properly coded
applications that free their opaque data on failure to register, as seen
in the following details:
Program terminated with signal 6, Aborted.
#0 0x00007fc45cba15d7 in raise
#1 0x00007fc45cba2cc8 in abort
#2 0x00007fc45cbe12f7 in __libc_message
#3 0x00007fc45cbe86d3 in _int_free
#4 0x00007fc45d8d292c in PyDict_Fini
#5 0x00007fc45d94f46a in Py_Finalize
#6 0x00007fc45d960735 in Py_Main
#7 0x00007fc45cb8daf5 in __libc_start_main
#8 0x0000000000400721 in _start
The double dereference of 'pyobj_cbData' is triggered in the following way:
(1) libvirt_virConnectDomainEventRegisterAny is invoked.
(2) the event is successfully added to the event callback list
(virDomainEventStateRegisterClient in
remoteConnectDomainEventRegisterAny returns 1 which means ok).
(3) when function remoteConnectDomainEventRegisterAny is hit,
network connection disconnected coincidently (or libvirtd is
restarted) in the context of function 'call' then the connection
is lost and the function 'call' failed, the branch
virObjectEventStateDeregisterID is therefore taken.
(4) 'pyobj_conn' is dereferenced the 1st time in
libvirt_virConnectDomainEventFreeFunc.
(5) 'pyobj_cbData' (refered to pyobj_conn) is dereferenced the
2nd time in libvirt_virConnectDomainEventRegisterAny.
(6) the double free error is triggered.
Resolve this by adding a @doFreeCb boolean in order to avoid calling the
freeCb in virObjectEventStateDeregisterID for any remote call failure in
a remoteConnect*EventRegister* API. For remoteConnect*EventDeregister* calls,
the passed value would be true indicating they should run the freecb if it
exists; whereas, it's false for the remote call failure path.
Patch based on the investigation and initial patch posted by
fangying <fangying1@huawei.com>.
virDomainXMLOption gains driver specific callbacks for parsing and
formatting save cookies.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
While checking for ABI stability, drivers might pose additional
checks that are not valid for general case. For instance, qemu
driver might check some memory backing attributes because of how
qemu works. But those attributes may work well in other drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
... with VIR_NET_GENERATED_MACV???_PREFIX, which is defined in
util/virnetdevmacvlan.h.
Since VIR_NET_GENERATED_PREFIX is used for plain tap devices, it is
renamed to VIR_NET_GENERATED_TAP_PREFIX and moved to virnetdev.h
This patch makes use of the virNetDevSetCoalesce() function to make
appropriate settings effective for devices that support them.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1414627
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
So far our code is full of the following pattern:
dom = virGetDomain(conn, name, uuid)
if (dom)
dom->id = 42;
There is no reasong why it couldn't be just:
dom = virGetDomain(conn, name, uuid, id);
After all, client domain representation consists of tuple (name,
uuid, id).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There is no "node driver" as there was before, drivers have to do
their own ACL checking anyway, so they all specify their functions and
nodeinfo is basically just extending conf/capablities. Hence moving
the code to src/conf/ is the right way to go.
Also that way we can de-duplicate some code that is in virsysfs and/or
virhostcpu that got duplicated during the virhostcpu.c split. And
Some cleanup is done throughout the changes, like adding the vir*
prefix etc.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
There is no reason for it not to be in the utils, all global symbols
under that file already have prefix vir* and there is no reason for it
to be part of DRIVER_SOURCES because that is just a leftover from
older days (pre-driver modules era, I believe).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Both QEMU and bhyve are using the same function for setting up the CPU
in virCapabilities, so de-duplicate it, save code and time, and help
other drivers adopt it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Along with video and VNC support, bhyve has introduced USB tablet
support as an input device. This tablet is exposed to a guest
as a device on an XHCI controller.
At present, tablet is the only supported device on the XHCI controller
in bhyve, so to make things simple, it's allowed to only have a
single XHCI controller with a single tablet device.
In detail, this commit:
- Introduces a new capability bit for XHCI support in bhyve
- Adds an XHCI controller and tabled support with 1:1 mapping
between them
- Adds a couple of unit tests
There are a number of functions in bhyve_capabilities.c that probe
hypervisor capabilities by executing the bhyve(1) binary with the
specific device arugment, checking error message (if any) and setting
proper capability bit. As those are extremely similar, move this logic
into a helper function and convert existing functions to use that.
* Extract filling bhyve capabilities from virBhyveDomainCapsBuild()
into a new function virBhyveDomainCapsFill() to make testing
easier by not having to mock firmware directory listing and
hypervisor capabilities probing
* Also, just presence of the firmware files is not sufficient
to enable os.loader.supported, hypervisor should support UEFI
boot too
* Add tests to domaincapstest for the main caps possible flows:
- when UEFI bootrom is supported
- when video (fbus) is supported
- neither of above is supported
Introduce config file support for the bhyve driver. The only available
setting at present is 'firmware_dir' for specifying a directory with
UEFI firmware files.
When creating host CPU definition usable with a given emulator, the CPU
should not be defined using an unsupported CPU model. The new @models
and @nmodels parameters can be used to limit CPU models which can be
used in the result.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The parameter can be used to request either VIR_CPU_TYPE_HOST (which has
been assumed so far) or VIR_CPU_TYPE_GUEST definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
cpuNodeData has always been followed by cpuDecode as no hypervisor
driver is really interested in raw CPUID data for a host CPU. Let's
create a new CPU driver API which returns virCPUDefPtr directly.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
bhyve supports 'gop' video device that allows clients to connect
to VMs using VNC clients. This commit adds support for that to
the bhyve driver:
- Introducr 'gop' video device type
- Add capabilities probing for the 'fbuf' device that's
responsible for graphics
- Update command builder routines to let users configure
domain's VNC via gop graphics.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Extend domain capabilities XML with the information about
available UEFI firmware files. It searches in the location
that the sysutils/bhyve-firmware FreeBSD port installs
files to.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Allow to boot using UEFI rather than using an external boot loader
such as bhyveload or grub-bhyve.
Also, make LPC PCI-ISA bridge handling more flexible as now it's
needed not only for serial ports, but for bootrom as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Implement the BHACE_CAP_LPC_BOOTROM capability by checking the stderr
output of 'bhyve -l bootrom'. If the bootrom option is unsupported, this
will contain the following output:
bhyve: invalid lpc device configuration 'bootrom'
On newer bhyve versions that do support specifying a bootrom image, the
standard help will be printed.
The new API is called virCPUDataFree. Individual CPU drivers are no
longer required to implement their own freeing function unless they need
to free architecture specific data from virCPUData.
Signed-off-by: Jiri Denemark <jdenemar@redhat.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
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).
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.
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.
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.
Since we don't actually reserve an entire slot at a time anymore, the
name of this function is just confusing, and it's almost identical in
operation to virDomainPCIAddressReserveNextAddr() anyway, so remove
the *Slot() function and replace calls to it with calls to *Addr(...,
-1).
When changing the metadata via virDomainSetMetadata, we now
emit an event to notify the app of changes. This is useful
when co-ordinating different applications read/write of
custom metadata.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We have couple of functions that operate over NULL terminated
lits of strings. However, our naming sucks:
virStringJoin
virStringFreeList
virStringFreeListCount
virStringArrayHasString
virStringGetFirstWithPrefix
We can do better:
virStringListJoin
virStringListFree
virStringListFreeCount
virStringListHasString
virStringListGetFirstWithPrefix
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
New line character in name of domain is now forbidden because it
mess virsh output and can be confusing for users.
Validation of name is done in drivers, after parsing XML to avoid
problems with dissappeared domains which was already created with
new-line char in name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Change the virDomainChrDef to use a pointer to 'source' and allocate
that pointer during virDomainChrDefNew.
This has tremendous "fallout" in the rest of the code which mainly
has to change source.$field to source->$field.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This function for some weird reason returns integer instead of
virDomainNetType type. It is important to return the correct type
so that we know what values we can expect.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Just like virDomainDefPostParseCallback has gained new
parseOpaque argument, we need to follow the logic with
virDomainDeviceDefPostParse.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We want to pass the proper opaque pointer instead of NULL to
virDomainDefParse and subsequently virDomainDefParseNode too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Some callers might want to pass yet another pointer to opaque
data to post parse callbacks. The driver generic one is not
enough because two threads executing post parse callback might
want to see different data (e.g. domain object pointer that
domain def belongs to).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
As bhyve currently doesn't use controller addressing and simply
uses 1 implicit controller for 1 disk device, the scheme looks the
following:
pci addrees -> (implicit controller) -> disk device
So in fact we identify disk devices by pci address of implicit
controller and just pass it this way to bhyve in a form:
-s pci_addr,ahci-(cd|hd),/path/to/disk
Therefore, we cannot use virDeviceInfoPCIAddressWanted() because it
does not expect that disk devices might need PCI address assignment.
As a result, if a disk was specified without address, it will not be
generated and domain will to start.
Until proper controller addressing is implemented in the bhyve
driver, force each disk to have PCI address generated if it was not
specified by user.
Allow to store driver specific data on a per-vcpu basis.
Move of the virDomainDef*Vcpus* functions was necessary as
virDomainXMLOptionPtr was declared below this block and I didn't want to
split the function headers.
A simple getopt-based argument parser is added for the /usr/sbin/bhyveload
command, loosely based on its argument parser.
The boot disk is guessed by iterating over all
disks and matching their sources. If any non-default arguments are found,
def->os.bootloaderArgs is set accordingly, and the bootloader is treated as a
custom bootloader.
Custom bootloader are supported by setting the def->os.bootloader and
def->os.bootloaderArgs accordingly
grub-bhyve is also treated as a custom bootloader. Since we don't get the
device map in the native format anyways, we can't reconstruct the complete
boot order. While it is possible to check what type the grub boot disk is by
checking if the --root argument is "cd" or "hd0,msdos1", and then just use the
first disk found, implementing the grub-bhyve argument parser as-is in the
grub-bhyve source would mean adding a dependency to argp or duplicating lots
of the code of argp. Therefore it's not really worth implementing that now.
Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
A simpe getopt-based argument parser is added for the /usr/sbin/bhyve command,
loosely based on its argument parser, which reads the following from the bhyve
command line string:
* vm name
* number of vcpus
* memory size
* the time offset (UTC or localtime)
* features:
* acpi
* ioapic: While this flag is deprecated in FreeBSD r257423, keep checking for
it for backwards compatibiility.
* the domain UUID; if not explicitely given, one will be generated.
* lpc devices: for now only the com1 and com2 are supported. It is required for
these to be /dev/nmdm[\d+][AB], and the slave devices are automatically
inferred from these to be the corresponding end of the virtual null-modem
cable: /dev/nmdm<N>A <-> /dev/nmdm<N>B
* PCI devices:
* Disks: these are numbered in the order they are found, for virtio and ahci
disks separately. The destination is set to sdX or vdX with X='a'+index;
therefore only 'z'-'a' disks are supported.
Disks are considered to be block devices if the path
starts with /dev, otherwise they are considered to be files.
* Networks: only tap devices are supported. Since it isn't possible to tell
the type of the network, VIR_DOMAIN_NET_TYPE_ETHERNET is assumed, since it
is the most generic. If no mac is specified, one will be generated.
Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
First, remove escaped newlines and split up the string into an argv-list for
the bhyve and loader commands, respectively. This is done by iterating over the
string splitting it by newlines, and then re-iterating over each line,
splitting it by spaces.
Since this code reuses part of the code of qemu_parse_command.c
(in bhyveCommandLine2argv), add the appropriate copyright notices.
Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
In preparation for moving all the CPU related APIs out of
the nodeinfo file, give them a virHostCPU name prefix.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In preparation for moving all the memory related APIs out of
the nodeinfo file, give them a virHostMem name prefix.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Nearly all the methods in the nodeinfo file are given a
'const char *sysfs_prefix' parameter to override the
default sysfs path (/sys/devices/system). Every single
caller passes in NULL for this, except one use in the
unit tests. Furthermore this parameter is totally
Linux-specific, when the APIs are intended to be cross
platform portable.
This removes the sysfs_prefix parameter and instead gives
a new method linuxNodeInfoSetSysFSSystemPath for use by
the test suite.
For two of the methods this hardcodes use of the constant
SYSFS_SYSTEM_PATH, since the test suite does not need to
override the path for thos methods.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virConnectOpenInternal method opens the libvirt client
config file and uses it to resolve things like URI aliases.
There may be driver specific things that are useful to
store in the config file too, so rather than have them
re-parse the same file, pass the virConfPtr down to the
drivers.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Rather than only assigning a PCI address when no address is given at
all, also do it when the config says that the address type is 'pci',
but it gives no address.
Bhyve supports ACPI shutdown by issuing SIGTERM signal to a bhyve
process.
Add the bhyveDomainShutdown() function and virBhyveProcessShutdown()
helper function that just sends SIGTERM to VM's bhyve process. If
a guest supports ACPI shutdown then process will be terminated and
this event will be noticed by the bhyve monitor code that will handle
setting proper status and clean up VM's resources by calling
virBhyveProcessStop().
Current implementation of domainDestroy for bhyve calls
virProcessKillPainfully() for the bhyve process and then
executes "bhyvectl --destroy".
This is wrong for two reasons:
* bhyvectl --destroy alone is sufficient because it terminates
the process
* virProcessKillPainfully() first sends SIGTERM and after few
attempts sends SIGKILL. As SIGTERM triggers ACPI shutdown that
we're not interested in, it creates an unwanted side effect in
domainDestroy.
Also, destroy monitor only after "bhyvectl --destroy" command succeeded
to avoid a case when the command fails and domain remains running, but
not being monitored anymore.
We had both and the only difference was that the latter also included
information about multifunction setting. The problem with that was that
we couldn't use functions made for only one of the structs (e.g.
parsing). To consolidate those two structs, use the one in virpci.h,
include that in domain_conf.h and add the multifunction member in it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The flags used to determine which devices could be plugged into which
controllers were quite confusing, as they tried to create classes of
connections, then put particular devices into possibly multiple
classes, while sometimes setting multiple flags for the controllers
themselves. The attempt to have a single flag indicate, e.g. that a
root-port or a switch-downstream-port could connect was not only
confusing, it was leading to a situation where it would be impossible
to specify exactly the right combinations for a new controller.
The solution is for the VIR_PCI_CONNECT_TYPE_* flags to have a 1:1
correspondence with each type of PCI controller, plus a flag for a PCI
endpoint device and another for a PCIe endpoint device (the only
exception to this is that pci-bridge and pcie-expander-bus controllers
have their upstream connection classified as
VIR_PCI_CONNECT_TYPE_PCI_DEVICE since they can be plugged into
*exactly* the same ports as any endpoint device). Each device then
has a single flag for connect type (plus the HOTPLUG flag if that
device can e hotplugged), and each controller sets the CONNECT bits
for all controllers that can be plugged into it, as well as for either
type of endpoint device that can be plugged in (and the HOTPLUG flag
if it can accept hotplugged devices).
With this change, it is *slightly* easier to understand the matching
of connections (as long as you remember that the flag for a
device/upstream-facing connection of a controller is the same as that
device's type, while the flags for a controller's downstream
connections is the OR of all device types that can be plugged into
that controller). More importantly, it will be possible to correctly
specify what can be plugged into a pcie-switch-expander-bus, when
support for it is added.
Introduce a helper to check supported device and domain config and move
the memory hotplug checks to it.
The advantage of this approach is that by default all new features are
considered unsupported by all hypervisors unless specifically changed
rather than the previous approach where every hypervisor would need to
declare that a given feature is unsupported.
Syntax-check fails with:
cppi: src/bhyve/bhyve_driver.h: line 26: not properly indented
cppi: src/bhyve/bhyve_driver.h: line 27: not properly indented
maint.mk: incorrect preprocessor indentation
Fix by properly indenting '#include's.
Pushed as trivial.
After 1036ddadb2 we use bhyveDriverGetCapabilities from other
sources too, not only from bhyve_driver.c. However, the function
was static so not properly expose to other files. In order to
expose it, we need to move couple of #include-s too.
Then, there has been a copy paste error in
virBhyveProcessReconnect: s/privconn/data->driver/.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The virDomainObjFormat and virDomainSaveStatus methods
both call into virDomainDefFormat, so should be providing
a non-NULL virCapsPtr instance.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
virDomainSaveConfig calls virDomainDefFormat which was setting the caps
to NULL, thus keeping the old behaviour (i.e. not looking at
netprefix). This patch adds the virCapsPtr to the function and allows
the configuration to be saved and skipping interface names that were
registered with virCapabilitiesSetNetPrefix().
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
And use the newly added caps->host.netprefix (if it exists) for
interface names that match the autogenerated target names.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Make bhyveload respect boot order as specified by os.boot section of the
domain XML or by "boot order" for specific devices. As bhyve does not
support a real boot order specification right now, it's just about
choosing a single device to boot from.
Our domain_conf.* files are big enough. Not only they contain XML
parsing code, but they served as a storage of all functions whose
name is virDomain prefixed. This is just wrong as it gathers not
related functions (and modules) into one big file which is then
harder to maintain. Split virDomainObjList module into a separate
file called virdomainobjlist.[ch].
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Current monitor code overrides domain object's privateData, e.g.
in virBhyveProcessStart():
vm->privateData = bhyveMonitorOpen(vm, driver);
where bhyveMonitorPtr() returns bhyveMonitorPtr.
This is not right thing to do, so make bhyveMonitorPtr
a part of the bhyveDomainObjPrivate struct and change related code
accordingly.
We have macros for both positive and negative string matching.
Therefore there is no need to use !STREQ or !STRNEQ. At the same
time as we are dropping this, new syntax-check rule is
introduced to make sure we won't introduce it again.
Signed-off-by: Ishmanpreet Kaur Khera <khera.ishman@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=871452
So, you want to create a domain from XML. The domain already
exists in libvirt's database of domains. It's okay, because name
and UUID matches. However, on domain startup, internal
representation of the domain is overwritten with your XML even
though we claim that the XML you've provided is a transient one.
The bug is to be found across nearly all the drivers.
Le sigh.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Bhyve as of r279225 (FreeBSD -CURRENT) or r284894 (FreeBSD 10-STABLE)
supports using UTC time offset via the '-u' argument to bhyve(8). By
default it's still using localtime.
Make the bhyve driver use UTC clock if it's requested by specifying
<clock offset='utc'> in domain XML and if the bhyve(8) binary supports
the '-u' flag.
Build with gcc 4.8 fails with:
bhyve/bhyve_monitor.c: In function 'bhyveMonitorIO':
bhyve/bhyve_monitor.c:51:18: error: missing initializer for field 'tv_sec' of 'const struct timespec' [-Werror=missing-field-initializers]
const struct timespec zerowait = {};
Explicitly initialize zerowait to fix the build.
Every domain that grabs a domain object to work over should
reference it to make sure it won't disappear meanwhile.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This needs to specified in way too many places for a simple validation
check. The ostype/arch/virttype validation checks later in
DomainDefParseXML should catch most of the cases that this was covering.
This patch adds code that parses and formats configuration for memory
devices.
A simple configuration would be:
<memory model='dimm'>
<target>
<size unit='KiB'>524287</size>
<node>0</node>
</target>
</memory>
A complete configuration of a memory device:
<memory model='dimm'>
<source>
<pagesize unit='KiB'>4096</pagesize>
<nodemask>1-3</nodemask>
</source>
<target>
<size unit='KiB'>524287</size>
<node>1</node>
</target>
</memory>
This patch preemptively forbids use of the <memory> device in individual
drivers so the users are warned right away that the device is not
supported.
Add a XML element that will allow to specify maximum supportable memory
and the count of memory slots to use with memory hotplug.
To avoid possible confusion and misuse of the new element this patch
also explicitly forbids the use of the maxMemory setting in individual
drivers's post parse callbacks. This limitation will be lifted when the
support is implemented.
Wikipedia's list of common misspellings [1] has a machine-readable
version. This patch fixes those misspellings mentioned in the list
which don't have multiple right variants (as e.g. "accension", which can
be both "accession" and "ascension"), such misspellings are left
untouched. The list of changes was manually re-checked for false
positives.
[1] https://en.wikipedia.org/wiki/Wikipedia:Lists_of_common_misspellings/For_machines
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
As there are two possible approaches to define a domain's memory size -
one used with legacy, non-NUMA VMs configured in the <memory> element
and per-node based approach on NUMA machines - the user needs to make
sure that both are specified correctly in the NUMA case.
To avoid this burden on the user I'd like to replace the NUMA case with
automatic totaling of the memory size. To achieve this I need to replace
direct access to the virDomainMemtune's 'max_balloon' field with
two separate getters depending on the desired size.
The two sizes are needed as:
1) Startup memory size doesn't include memory modules in some
hypervisors.
2) After startup these count as the usable memory size.
Note that the comments for the functions are future aware and document
state that will be present after a few later patches.
Return 0 instead of ERR_NO_SUPPORT in each driver
where we don't support managed save or -1 if
the domain does not exist.
This avoids spamming daemon logs when 'virsh dominfo' is run.
https://bugzilla.redhat.com/show_bug.cgi?id=1095637
For stateless, client side drivers, it is never correct to
probe for secondary drivers. It is only ever appropriate to
use the secondary driver that is associated with the
hypervisor in question. As a result the ESX & HyperV drivers
have both been forced to do hacks where they register no-op
drivers for the ones they don't implement.
For stateful, server side drivers, we always just want to
use the same built-in shared driver. The exception is
virtualbox which is really a stateless driver and so wants
to use its own server side secondary drivers. To deal with
this virtualbox has to be built as 3 separate loadable
modules to allow registration to work in the right order.
This can all be simplified by introducing a new struct
recording the precise set of secondary drivers each
hypervisor driver wants
struct _virConnectDriver {
virHypervisorDriverPtr hypervisorDriver;
virInterfaceDriverPtr interfaceDriver;
virNetworkDriverPtr networkDriver;
virNodeDeviceDriverPtr nodeDeviceDriver;
virNWFilterDriverPtr nwfilterDriver;
virSecretDriverPtr secretDriver;
virStorageDriverPtr storageDriver;
};
Instead of registering the hypervisor driver, we now
just register a virConnectDriver instead. This allows
us to remove all probing of secondary drivers. Once we
have chosen the primary driver, we immediately know the
correct secondary drivers to use.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virDomainDefineXMLFlags and virDomainCreateXML APIs both
gain new flags allowing them to be told to validate XML.
This updates all the drivers to turn on validation in the
XML parser when the flags are set
The virDomainDefParse* and virDomainDefFormat* methods both
accept the VIR_DOMAIN_XML_* flags defined in the public API,
along with a set of other VIR_DOMAIN_XML_INTERNAL_* flags
defined in domain_conf.c.
This is seriously confusing & error prone for a number of
reasons:
- VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_XML_MIGRATABLE and
VIR_DOMAIN_XML_UPDATE_CPU are only relevant for the
formatting operation
- Some of the VIR_DOMAIN_XML_INTERNAL_* flags only apply
to parse or to format, but not both.
This patch cleanly separates out the flags. There are two
distint VIR_DOMAIN_DEF_PARSE_* and VIR_DOMAIN_DEF_FORMAT_*
flags that are used by the corresponding methods. The
VIR_DOMAIN_XML_* flags received via public API calls must
be converted to the VIR_DOMAIN_DEF_FORMAT_* flags where
needed.
The various calls to virDomainDefParse which hardcoded the
use of the VIR_DOMAIN_XML_INACTIVE flag change to use the
VIR_DOMAIN_DEF_PARSE_INACTIVE flag.
Reboot requires more sophistication and is left as a future work item --
but at least part of the plumbing is in place.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Rather than just picking the first CD (or failing that, HDD) we come
across, if the user has picked a boot device ordering with <boot
order=''>, respect that (and just try to boot the lowest-index device).
Adds two sets of tests to bhyve2xmlargv; 'grub-bootorder' shows that we
pick a user-specified device over the first device in the domain;
'grub-bootorder2' shows that we pick the first (lowest index) device.
This enables booting interactive GRUB menus (e.g. install CDs) with
libvirt-bhyve.
Caveat: A terminal other than the '--console' option to 'virsh start'
(e.g. 'cu -l /dev/nmdm0B -s 115200') must be used to connect to
grub-bhyve because the bhyve loader path is synchronous and must occur
before the VM actually starts.
Changing the bhyveProcessStart logic around to accommodate '--console'
for interactive loader use seems like a significant project and probably
not worth it, if UEFI/BIOS support for bhyve is "coming soon."
We still default to bhyveloader(1) if no explicit bootloader
configuration is supplied in the domain.
If the /domain/bootloader looks like grub-bhyve and the user doesn't
supply /domain/bootloader_args, we make an intelligent guess and try
chainloading the first partition on the disk (or a CD if one exists,
under the assumption that for a VM a CD is likely an install source).
Caveat: Assumes the HDD boots from the msdos1 partition. I think this is
a pretty reasonable assumption for a VM. (DrvBhyve with Bhyveload
already assumes that the first disk should be booted.)
I've tested both HDD and CD boot and they seem to work.
To prepare for introducing a single global driver, rename the
virDriver struct to virHypervisorDriver and the registration
API to virRegisterHypervisorDriver()
Update bhyveBuildDiskArgStr to support volumes:
- Make virBhyveProcessBuildBhyveCmd and
virBhyveProcessBuildLoadCmd take virConnectPtr as the
first argument instead of bhyveConnPtr as virConnectPtr is
needed for virStorageTranslateDiskSourcePool,
- Add virStorageTranslateDiskSourcePool call to
virBhyveProcessBuildBhyveCmd and
virBhyveProcessBuildLoadCmd,
- Allow disks of type VIR_STORAGE_TYPE_VOLUME
Add support for CDROM devices for bhyve driver using
bhyve(8)'s 'ahci-cd' device type.
As bhyve currently does not support media insertion at runtime,
disallow to start a domain with an empty source path for cdrom
devices.
https://bugzilla.redhat.com/show_bug.cgi?id=1122205
Although the edits were changing in-memory XML, it was not flushed
to disk; so unless some other action changes XML, a libvirtd restart
would lose the changed information.
* src/conf/domain_conf.c (virDomainObjSetMetadata): Add parameter,
to save live status across restarts.
(virDomainSaveXML): Allow for test driver.
* src/conf/domain_conf.h (virDomainObjSetMetadata): Adjust
signature.
* src/bhyve/bhyve_driver.c (bhyveDomainSetMetadata): Adjust caller.
* src/lxc/lxc_driver.c (lxcDomainSetMetadata): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSetMetadata): Likewise.
* src/test/test_driver.c (testDomainSetMetadata): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Try to reconnect to the running domains after libvirtd restart. To
achieve that, do:
* Save domain state
- Modify virBhyveProcessStart() to save domain state to the state
dir
- Modify virBhyveProcessStop() to cleanup the pidfile and the state
* Detect if the state information loaded from the driver's state
dir matches the actual state. Consider domain active if:
- PID it points to exist
- Process title of this PID matches the expected one with the
domain name
Otherwise, mark the domain as shut off.
Note: earlier development bhyve versions before FreeBSD 10.0-RELEASE
didn't set proctitle we expect, so the current code will not detect
it. I don't plan adding support for this unless somebody requests
this.
Replace:
if (virBufferError(&buf)) {
virBufferFreeAndReset(&buf);
virReportOOMError();
...
}
with:
if (virBufferCheckError(&buf) < 0)
...
This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)
So far, we only report an error if formatting the siblings bitmap
in NUMA topology fails.
Be consistent and always report error in virCapabilitiesFormatXML.
The new VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE flag for
virConnectCompareCPU can be used to get an error
(VIR_ERR_CPU_INCOMPATIBLE) describing the incompatibility instead of the
usual VIR_CPU_COMPARE_INCOMPATIBLE return code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When CPU comparison APIs return VIR_CPU_COMPARE_INCOMPATIBLE, the caller
has no clue why the CPU is considered incompatible with host CPU. And in
some cases, it would be nice to be able to get such info in a client
rather than having to look in logs.
To achieve this, the APIs can be told to return VIR_ERR_CPU_INCOMPATIBLE
error for incompatible CPUs and the reason will be described in the
associated error message.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
For future work we want to get info for not only the free memory
but overall memory size too. That's why the function must have
new signature too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When virBhyveProcessStart() fails, it tries to unload
a guest that could have been already loaded using
bhyveload(8) to make sure not to leave it hanging in memory.
However, we could fail before loading a VM into memory,
so 'bhyvectl --destroy' command will fail and print
an error message that looks confusing to users.
So ignore errors when running this in cleanup.
virBhyveProcessStart() calls bhyveNetCleanup() if it fails. However,
it might fail earlier than networks are allocated, so modify
bhyveNetCleanup() to check if net->ifname is not NULL before
going further with the cleanup.
bhyveBuildNetArgStr() calls virNetDevTapCreateInBridgePort() and
passes tapfd = NULL, but tapfdSize = 1. That is wrong, because
if virNetDevTapCreateInBridgePort() crashes after successfully
creating a TAP device, it'll jump to 'error' label, that
loops over tapfd and calls VIR_FORCE_CLOSE:
for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)
In that case we get a segfault.
As the bhyve code doesn't use tapfd, pass NULL and set tapfdSize to 0.
Automatically allocate PCI addresses for devices instead
of hardcoding them in the driver code. The current
allocation schema is to dedicate an entire slot for each devices.
Also, allow having arbitrary number of devices.
In a number of places in the bhyve driver, virObjectUnlock()
is called with an arg without check if the arg is non-NULL, which
could result in passing NULL value and a warning like:
virObjectUnlock:340 : Object 0x0 ((unknown)) is not a virObjectLockable instance
* src/bhyve/bhyve_driver.c (bhyveDomainGetInfo)
(bhyveDomainGetState, bhyveDomainGetAutostart)
(bhyveDomainSetAutostart, bhyveDomainIsActive)
(bhyveDomainIsPersistent, bhyveDomainGetXMLDesc)
(bhyveDomainUndefine, bhyveDomainLookupByUUID)
(bhyveDomainLookupByName, bhyveDomainLookupByID)
(bhyveDomainCreateWithFlags, bhyveDomainOpenConsole):
Check if arg is not NULL before calling virObjectUnlock on it.
Add a helper function virBhyveGetDomainTotalCpuStats() to
obtain process CPU time using kvm (kernel memory interface)
and use it to set cpuTime field of the virDomainInfo struct in
bhyveDomainGetInfo().
- do not lose new definition for an active domain
- do not leak oldDef
- do not set dom->id if virDomainSaveConfig() fails
- do not call virObjectUnlock(vm) if vm is NULL
Implement bhyveDomainCreateXML function.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Fix incorrect ATTRIBUTE_NONNULL usage introduced in 17b17565
which caused build failure:
bhyve/bhyve_driver.c:127:48: error: expected ')'
bhyveDriverGetCapabilities(bhyveConnPtr driver ATTRIBUTE_NONNULL)
^
bhyve/bhyve_driver.c:127:27: note: to match this '('
bhyveDriverGetCapabilities(bhyveConnPtr driver ATTRIBUTE_NONNULL)
Pushed under the build breaker rule.
At the beginning of the function we gain a reference to the driver
capabilities. Then, we call format function (*) which if failed, unref
over caps is called. Then, at the end another unref occurs.
* - Moreover, the format was not called over gained caps, but over
privconn->caps directly which is not allowed anymore.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The driver passed as the only argument to the function should never be
NULL so there's no need to check it. After removing it, the whole
function collapses to a single line doing ref over driver
capabilities.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since b15a2bbd we have the new bhyve_capabilities.[ch] files.
However, the copyright is held by both Roman and Semihalf.
Signed-off-by: Michal Privoznik <mprivozn@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.
On failures, virBhyveProcessStart() does not cleanup network
interfaces that could be created by virBhyveProcessBuildBhyveCmd(),
which results in a leaked tap device.
To fix that, extract network cleanup code to bhyveNetCleanup()
and use it in cleanup stage of virBhyveProcessStart().
To ease mocking for bhyve unit tests move virBhyveTapGetRealDeviceName()
out of bhyve_command.c to virnetdevtap and rename it to
virNetDevTapGetRealDeviceName().
We use virBhyveTapGetRealDeviceName() to map network interface name
to a real device path, trying to open possible devices and getting
names by ioctl.
Make it skip devices that fail to open with EBUSY because they're
most likely already used by other VMs.
Part of a series of cleanups to use new accessor methods.
* src/bhyve/bhyve_command.c (bhyveBuildDiskArgStr)
(virBhyveProcessBuildLoadCmd): Use accessors.
Signed-off-by: Eric Blake <eblake@redhat.com>
New functionalities:
- connectGetMaxVcpus - on bhyve hardcode this value to 16
- nodeGetFreeMemory - do not use physmem_get on FreeBSD, since
it might get wrong value on systems with
more than 100GB of RAM
- nodeGetCPUMap - wrapper only for mapping function, currently not
supported by FreeBSD
- nodeSet/GetMemoryParameters - wrapper only for future improvements,
currently not supported by FreeBSD
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>
Addition of the hostbridge device was mistakenly placed to
bhyveBuildNetArgStr(). This could result in hostbridge device not being
added to the commandline if there are no network devices specified, but
hostbridge device should be added unconditionally.
Fix by placing it to virBhyveProcessBuildBhyveCmd().
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>
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>
At this point it has a limited functionality and is highly
experimental. Supported domain operations are:
* define
* start
* destroy
* dumpxml
* dominfo
It's only possible to have only one disk device and only one
network, which should be of type bridge.
