Commit 2aa167ca tried to fix the DBus interaction code to allow
callers to use native types instead of 4-byte bools. But in
fixing the issue, I missed the case of an arrayref; Conrad Meyer
shows the following valid complaint issued by clang:
CC util/libvirt_util_la-virdbus.lo
util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
But fixing that points out that we have NEVER supported arrayrefs
of sub-int types (byte, i16, u16, and now bool). Again, while raw
types promote, arrays do not; so the macros HAVE to deal with both
size possibilities rather than assuming that an arrayref uses the
same sizing as the promoted raw type.
Obviously, our testsuite wasn't covering as much as it should have.
* src/util/virdbus.c (GET_NEXT_VAL): Also fix array cases.
(SET_NEXT_VAL): Fix uses of sub-int arrays.
* tests/virdbustest.c (testMessageArray, testMessageArrayRef):
Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit 6fcddfcd refactored job statistics but missed the jobinfo type updated
in qemuDomainGetJobInfo. After this patch, we can use virDomainGetJobInfo to
get jobinfo type again.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit 'c264eeaa' didn't do the prerequisite 'make syntax-check' before
pushing. There was a <tab> in the whitespace for the comment. Replaced
with spaces and aligned.
pushed as build breaker since Jenkins complained loudly
The typical case where we had a problem is with such a filesystem
definition as created by virt-sandbox-service:
<filesystem type='bind' accessmode='passthrough'>
<source dir='/var/lib/libvirt/filesystems/mysshd/var'/>
<target dir='/var'/>
</filesystem>
In this case, we don't want to unmount the /var subtree or we may
loose the access to the source folder.
Resolving symlinks can fail before mounting any file system if one file
system depends on another being mounted. Symlinks are now resolved in
two passes:
* Before any file system is mounted, but then we are more gentle if
the source path can't be accessed
* Right before mounting a file system, so that we are sure that we
have the resolved path... but then if it can't be accessed we raise
an error.
Due to a change (or bug?) in ip link implementation, the command
'ip link add vnet0...'
is forced into
'ip link add name vnet0...'
The changed command also works on older versions of iproute2, just the
'name' parameter has been made mandatory.
Add attribute to set vgamem_mb parameter of QXL device for QEMU. This
value sets the size of VGA framebuffer for QXL device. Default value in
QEMU is 8MB so reuse it also in libvirt to not break things.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
So far we didn't have any option to set video memory size for qemu video
devices. There was only the vram (ram for QXL) attribute but it was valid
only for the QXL video device.
To provide this feature to users QEMU has a dedicated device attribute
called 'vgamem_mb' to set the video memory size. We will use the 'vram'
attribute for setting video memory size for other QEMU video devices.
For the cirrus device we will ignore the vram value because it has
hardcoded video size in QEMU.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
QEMU has two different type of QXL display device. The first "qxl-vga"
is for primary video device and second "qxl" is for secondary video
device.
There are also two different ways how to specify those devices on qemu
command line, the first one and obsolete is using "-vga" option and the
current new one is using "-device" option. The "-vga" could be used only
to setup primary video device, so the "-vga qxl" equal to
"-device qxl-vga". Unfortunately the "-vga qxl" doesn't support setting
additional parameters for the device and "-global" option must be used
for this purpose. It's mandatory to use "-global qxl-vga...." to set the
parameters of primary video device previously defined with "-vga qxl".
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The vram attribute was introduced to set the video memory but it is
usable only for few hypervisors excluding QEMU/KVM and the old XEN
driver. Only in case of QEMU the vram was used for QXL.
This patch updates the documentation to reflect current code in libvirt
and also changes the cases when we will set the default vram attribute.
It also fixes existing strange default value for VGA devices 9MB to 16MB
because the video ram should be rounded to power of two.
The change of default value could affect migrations but I found out that
QEMU always round the video ram to power of two internally so it's safe
to change the default value to the next closest power of two and also
silently correct every domain XML definition. And it's also safe because
we don't pass the value to QEMU.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1076098
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
There are two special cases, if the input number is 0 or the number is
larger then 2^31 (for 32bit unsigned int). For the special cases the
return value is 0 because they cannot be rounded.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Get mounted filesystems list, which contains hardware info of disks and its
controllers, from QEMU guest agent 2.2+. Then, convert the hardware info
to corresponding device aliases for the disks.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
virDomainGetFSInfo returns a list of filesystems information mounted in the
guest, which contains mountpoints, device names, filesystem types, and
device aliases named by libvirt. This will be useful, for example, to
specify mountpoints to fsfreeze when taking snapshot of a part of disks.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
As qemu is now able to notify us about change of the channel state used
for communication with the guest agent we now can more precisely track
the state of the guest agent.
To allow notifying management apps this patch implements a new event
that will be triggered on changes of the guest agent state.
Improve the monitor function to also retrieve the guest state of
character device (if provided) so that we can refresh the state of
virtio-serial channels and perhaps react to changes in the state in
future patches.
This patch changes the returned data from qemuMonitorGetChardevInfo to
return a structure containing the pty path and the state for all the
character devices.
The change to the testsuite makes sure that the data is parsed
correctly.
This patch contains three domain cleanup improvements in the migration
finish phase, ensuring a domain is properly disposed when a failure is
detected or the migration is cancelled.
The check for virDomainObjIsActive is moved to libxlDomainMigrationFinish,
where cleanup can occur if migration failed and the domain is inactive.
The 'cleanup' label was missplaced in libxlDomainMigrationFinish, causing
a migrated domain to remain in the event of an error or cancelled migration.
In cleanup, the domain was not removed from the driver's list of domains.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
During the perform phase of migration, the domain is started on
the dst host in a running state if VIR_MIGRATE_PAUSED flag is not
specified. In the finish phase, the domain is also unpaused if
VIR_MIGRATE_PAUSED flag is unset. I've noticed this second unpause
fails if the domain was already unpaused following the perform phase.
This patch changes the perform phase to always start the domain
paused, and defers unpausing, if requested, to the finish phase.
Unpausing should occur in the finish phase anyhow, where the domain
can be properly destroyed if the perform phase fails and migration
is cancelled.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Moving data reception of the perform phase of migration to a
thread introduces a race with the finish phase, where checking
if the domain is active races with the thread finishing the
perform phase. The race is easily solved by acquiring a job in
the finish phase, which must wait for the perform phase job to
complete.
While wrapping the finish phase in a job, noticed the virDomainObj
was being unlocked in a callee - libxlDomainMigrationFinish. Move
the unlocking to libxlDomainMigrateFinish3Params, where the lock
is acquired.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The libxl driver receives migration data within an IO callback invoked
by the event loop, effectively disabling the event loop while migration
occurs.
This patch moves receving of the migration data to a thread. The
incoming connection is still accepted in the IO callback, but control
is immediately returned to the event loop after spawning the thread.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Specifying an explicit path to pygrub (e.g. BINDIR "/pygrub") only works if
Xen and libvirt happen to be installed to the same prefix. A more flexible
approach is to simply specify "pygrub" which will cause libxl to use the
correct path which it knows (since it is built with the same prefix as pygrub).
This is particular problematic in the Debian packaging, since the Debian Xen
package relocates pygrub into a libexec dir, however I think this change makes
sense upstream.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
To be able to express some use cases of the RBD backing with libvirt, we
need to be able to specify a config file for the RBD client to qemu as
that is one of the commonly used options.
Some storage systems have internal support for snapshots. Libvirt should
be able to select a correct snapshot when starting a VM.
This patch adds a XML element to select a storage source snapshot for
the RBD protocol which supports this feature.
As we now have a common function to parse backing store string for RBD
backing store we can reuse it in the backing store walker so that we
don't fail on files backed by RBD storage.
This patch also adds a few tests to verify that the parsing works as
expected.
To allow reuse this non-trivial parser code in the backing store parser
this part of the command line parser needs to be split out into a
separate funciton.
Instead of splitting out various fields, pass the complete structure and
let the function pick various things of it.
As one of the callers isn't using virStorageSourcePtr to store the data,
this patch adds glue code that fills the data into a dummy
virStorageSourcePtr before calling the func.
This change will help when adding new fields that need output processing
in the future.
If there are no hosts for a storage source virStorageSourceCopy and
virStorageSourceNewFromBackingRelative would try to copy them anyways.
As the success of virStorageNetHostDefCopy is determined by returning
a pointer and malloc of 0 elements might return NULL according to the
implementation, the result of the copy function may vary.
Fix this by copying the hosts array only if there are hosts defined.
As we now have a deep copy function for struct virStorageSource add a
notice that extensions of the structure require also appropriate changes
to the virStorageSourceCopy func.
New qemu added a new event that is emitted when a virtio serial channel
is opened in the guest OS. This allows us to update the state of the
port in the output-only XML element.
This patch implements the monitor callbacks and necessary handlers to
update the state in the definition.
To track state of virtio channels this patch adds a new output-only
attribute called 'state' to the <target> element of virtio channels.
This will be later populated with the guest state of the channel.
To unify future additions that require information from "query-chardev"
rename qemuMonitorGetPtyPaths and friends to qemuMonitorGetChardevInfo
and move the allocation of the returned hash into the top level
function.
When creating a disk image snapshot the libvirt code would blindly copy
the parents label to the newly created image. This runs into problems
when you start a VM from an image hosted on NFS (or other storage system
that doesn't support selinux labels) and the snapshot destination is on
a storage system that does support selinux labels. Libvirt's code in
that case generates a different security label for the image hosted on
NFS. This label is valid only for NFS images and doesn't allow access in
case of a locally stored image.
To fix this issue libvirt needs to refrain from copying security
information in cases where the default domain seclabel is a better
choice.
This patch repurposes the now unused @force argument of
virStorageSourceInitChainElement to denote whether a copy of the
security labelling stuff should be attempted or not. This allows to
fine-control the copy operation for cases where we need to keep the
label of the old disk vs. the cases where we need to keep the label
unset to use the default domain imagelabel.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1151718
Commit c0e7022 breaks on a machine that lacks dbus headers:
In file included from util/virdbus.c:24:0:
util/virdbuspriv.h:31:3: error: unknown type name 'dbus_int16_t'
* src/util/virdbuspriv.h (DBusBasicValue): Only provide fallback
when dbus is compiled.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1152382
When libvirt create's the vport (VPORT_CREATE) for the vHBA, there isn't
enough "time" between the creation and the running of the following
backend->refreshPool after a backend->startPool in order to find the LU's.
Population of LU's happens asynchronously when udevEventHandleCallback
discovers the "new" vHBA port. Creation of the infrastructure by udev
is an iterative process creating and discovering actual storage devices and
adjusting the environment.
Because of the time it takes to discover and set things up, the backend
refreshPool call done after the startPool call will generally fail to
find any devices. This leaves the newly started pool appear empty when
querying via 'vol-list' after startup. The "workaround" has always been
to run pool-refresh after startup (or any time thereafter) in order to
find the LU's. Depending on how quickly run after startup, this too may
not find any LUs in the pool. Eventually though given enough time and
retries it will find something if LU's exist for the vHBA.
This patch adds a thread to be executed after the VPORT_CREATE which will
attempt to find the LU's without requiring the external run of refresh-pool.
It does this by waiting for 5 seconds and searching for the LU's. If any
are found, then the thread completes; otherwise, it will retry once more
in another 5 seconds. If none are found in that second pass, the thread
gives up.
Things learned while investigating this... No need to try and fill the
pool too quickly or too many times. Over the course of creation, the udev
code may 'add', 'change', and 'delete' the same device. So if the refresh
code runs and finds something, it may display it only to have a subsequent
refresh appear to "lose" the device. The udev processing doesn't seem to
have a way to indicate that it's all done with the creation processing of a
newly found vHBA. Only the Lone Ranger has silver bullets to fix everything.
Fix a problem in the getBlockDevice and processLU where retval initialized
to zero causing some failures to erroneously continue through to the
virStorageBackendSCSINewLun with an attempt to find a path for "/dev/(null)".
This would fail approximately 10 seconds later with debug message:
virStorageBackendSCSINewLun:203 :
No stable path found for '/dev/(null)' in '/dev/disk/by-path'
The root cause of the issue is for many /sys/bus/scsi/devices/<lun path>
there is no "block*" device found for the vHBA's, where "<lun path>" are
the various paths created for the vHBA, such as "17:0:0:0", "17:0:1:0",
"17:0:2:0", "17:0:3:0", etc. If the block device isn't found, then the
directory should just be ignored rather than attempting to process it.
The bug was that in getBlockDevice the assumption was "block" would exist
and either getNewStyleBlockDevice or getOldStyleBlockDevice would fill in
@block_device. However, if 'block*' doesn't exist, then the code returned
NULL for block_device *and* a good (zero) retval value. This caused the
processLU code to attempt the virStorageBackendSCSINewLun which failed
"at some point in time" in the future.
After this change - on test system the refresh-pool didn't have a noticable
pause of about 20 seconds - it completed within a second since no longer
was there an attempt/need to find "/dev/(null)".
Additionally, the virStorageBackendSCSIFindLU's shouldn't be declaring
found unless the processLU actually returns success. This will be
important in the followup patch which relies on whether a LU was found.
Compilation on a RHEL 5 host failed, due to the older dbus headers
present on that machine, and triggered by commit 2aa167ca:
util/virdbus.c: In function 'virDBusMessageIterDecode':
util/virdbus.c:952: error: 'DBusBasicValue' undeclared (first use in this function)
* m4/virt-dbus.m4 (LIBVIRT_CHECK_DBUS): Check for DBusBasicValue.
* src/util/virdbuspriv.h (DBusBasicValue): Provide fallback.
Signed-off-by: Eric Blake <eblake@redhat.com>
getsockopt(sock->fd, SOL_SOCKET, SO_PEERCRED, ...) sets the pid to 0
when the process that opens the connection is in another container.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Commit dc33e6e4 caused older platforms like Fedora 20 to emit
scary log messages at startup:
2014-11-19 23:12:58.800+0000: 28906: error : virCommandWait:2532 : internal error: Child process (/usr/sbin/iptables -w -L -n) unexpected exit status 2: iptables v1.4.19.1: unknown option "-w"
Try `iptables -h' or 'iptables --help' for more information.
Since we are probing and expect to handle the case where -w is not
supported, we should not let virCommand log it as an error.
* src/util/virfirewall.c (virFirewallCheckUpdateLock): Handle
non-zero status ourselves.
Signed-off-by: Eric Blake <eblake@redhat.com>
Oops, I forgot to squash one more instance of the same check in the
previous commit (v1.2.10-144-g52691f9).
https://bugzilla.redhat.com/show_bug.cgi?id=1147331
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Any attempt to start a tunnelled migration with libvirtd that supports
RDMA migration (specifically commit v1.2.8-226-ged22a47) crashes
libvirtd on the destination host.
The crash is inevitable because qemuMigrationPrepareAny is always called
with NULL protocol in case of tunnelled migration.
https://bugzilla.redhat.com/show_bug.cgi?id=1147331
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
As discussed on the upstream list, it's better not to make this
kind of predictions in libvirt. It may happen that qemu learns
how to enable OVMF on other architectures too and we shouldn't
try to chase that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, we are whitelisting architectures, that we know how to run
OVMF on. So far, only x86_64 was enabled. However, looking at qemu
code, the same commandline can be used to enable OVMF for armv7l and
aarch64.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
I noticed this while working on qemuDomainGetBlockInfo. Assigning
a bool value to an int variable compiles fine, but raises red flags
on the maintenance front as it becomes too easy to assign -1 or 2
or any other non-bool value to the same variable.
* cfg.mk (sc_prohibit_int_assign_bool): New rule.
* src/conf/snapshot_conf.c (virDomainSnapshotRedefinePrep): Fix
offenders.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo)
(qemuDomainSnapshotCreateXML): Likewise.
* src/test/test_driver.c (testDomainSnapshotAlignDisks):
Likewise.
* src/util/vircgroup.c (virCgroupSupportsCpuBW): Likewise.
* src/util/virpci.c (virPCIDeviceBindToStub): Likewise.
* src/util/virutil.c (virIsCapableVport): Likewise.
* tools/virsh-domain-monitor.c (cmdDomMemStat): Likewise.
* tools/virsh-domain.c (cmdBlockResize, cmdScreenshot)
(cmdInjectNMI, cmdSendKey, cmdSendProcessSignal)
(cmdDetachInterface): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Use of an 'int' to represent a 'bool' value is confusing. Just
because dbus made the mistake of cementing their 4-byte wire
format of dbus_bool_t into their API doesn't mean we have to
repeat the mistake. With a little bit of finesse, we can
guarantee that we provide a large-enough value to the DBus
code, while still copying only the relevant one-byte bool
to the client code, and isolate the rest of our code base from
the DBus stupidity.
* src/util/virdbus.c (GET_NEXT_VAL): Add parameter.
(virDBusMessageIterDecode): Adjust all clients.
* src/util/virpolkit.c (virPolkitCheckAuth): Use nicer type.
* tests/virdbustest.c (testMessageSimple, testMessageStruct):
Test new behavior.
Signed-off-by: Eric Blake <eblake@redhat.com>
This function returned non-inactive domains instead of active
domains. This broke virConnectNumOfDefinedDomains() and
virConnectListDefinedDomains() functions.
Ethernet interfaces in libvirt currently do not support bandwidth setting.
For example, following xml file for an interface will not apply these
settings to corresponding qdiscs.
<interface type="ethernet">
<mac address="02:36:1d:18:2a:e4"/>
<model type="virtio"/>
<script path=""/>
<target dev="tap361d182a-e4"/>
<bandwidth>
<inbound average="984" peak="1024" burst="64"/>
<outbound average="2000" peak="2048" burst="128"/>
</bandwidth>
</interface>
Signed-off-by: Anirban Chakraborty <abchak@juniper.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For some reason, commit id '72b4151f' triggered a Coverity uninitialized
'reply' variable check when referenced within the for loop.
It seems Coverity doesn't know that flags will have to be either AFFECT_LIVE
or AFFECT_CONFIG after the virDomainLiveConfigHelperMethod call.
By adding a "sa_assert()" to confirm that fact, Coverity is happy again.
https://bugzilla.redhat.com/show_bug.cgi?id=1164080
After a disk is hotunplugged a subsequent call to qemuDomainGetBlockIoTune
to get the --config settings of that disk will fail because the disk is no
longer found by qemuDiskPathToAlias causing an unexpected failure.
Since only the --live flag needs to have the disk device pointer, move the
fetch inside the (flags & VIR_DOMAIN_AFFECT_LIVE) condition. This will also
affect the results if no flags are provided or the --current flag is provided.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Seems the 'size_iops_sec' was a late add and the checks for whether
the field was defined, but unsupported and the maximum size of the
field were not being made.
Also, adjust blkdeviotune support error message for grammar, spelling
(paramater), and remove the "(need QEMU 1.7 or superior)". None of
our other similar error messages list which QEMU version is required.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rule sc_prohibit_newline_at_end_of_diagnostic for syntax-check does
check for passing strings ending with '\n' two lines after known
functions. This is, of course subject to false positives, so for the
sake of future changes, trick that syntax-check by adding one more line
with a comment.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
A previous commit introduced use of locking with invocation
of iptables in the viriptables.c module
commit ba95426d6f
Author: Serge Hallyn <serge.hallyn@ubuntu.com>
Date: Fri Nov 1 12:36:59 2013 -0500
util: use -w flag when calling iptables
This only ever had effect with the virtual network driver,
as it was not wired up into the nwfilter driver. Unfortunately
in the firewall refactoring the use of the -w flag was
accidentally lost.
This patch introduces it to the virfirewall.c module so that
both the virtual network and nwfilter drivers will be using
it. It also ensures that the equivalent --concurrent flag
to ebtables is used.
Since QEMU 1.2.0, we switched to QMP probing instead of parsing -help
(and other commands, such as -cpu ?) output. However, if QMP probing
failed, we still tried starting QEMU with various options and parsing
the output, which was guaranteed to fail because the output changed.
Let's just refuse parsing -help for QEMU >= 1.2.0.
https://bugzilla.redhat.com/show_bug.cgi?id=1160318
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
We used to set migration capabilities only when a user asked for them in
flags. This is fine when migration succeeds since the QEMU process is
killed in the end but in case migration fails or if it's cancelled, some
capabilities may remain turned on with no way to turn them off. To fix
that, migration capabilities have to be turned on if requested but
explicitly turned off in case they were not requested but QEMU supports
them.
https://bugzilla.redhat.com/show_bug.cgi?id=1163953
Signed-off-by: Jiri Denemark <jdenemar@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.
When user calls setmem on a running LXC machine, we do update its cgroup
entry, however we neither update domain's runtime XML nor
we update our internal structures and this patch fixes it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1131919
Commit 6e5c79a1 tried to fix deadlock between nwfilter{Define,Undefine}
and starting of guest, but this same deadlock exists for
updating/attaching network device to domain.
The deadlock was introduced by removing global QEMU driver lock because
nwfilter was counting on this lock and ensure that all driver locks are
locked inside of nwfilter{Define,Undefine}.
This patch extends usage of virNWFilterReadLockFilterUpdates to prevent
the deadlock for all possible paths in QEMU driver. LXC and UML drivers
still have global lock.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143780
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
In one of my previous patches (3a3c3780b) I've tried to fix the
problem of nvram path disappearing on a domain that's been
started and shut down again. I fixed this by explicitly saving
domain's config file. However, I did a bit of clumsy without
realizing we have a transient domains for which we don't save the
config file. Hence, any domain using UEFI became persistent.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160926
Introduce a 'managed' attribute to allow libvirt to decide whether to
delete a vHBA vport created via external means such as nodedev-create.
The code currently decides whether to delete the vHBA based solely on
whether the parent was provided at creation time. However, that may not
be the desired action, so rather than delete and force someone to create
another vHBA via an additional nodedev-create allow the configuration of
the storage pool to decide the desired action.
During createVport when libvirt does the VPORT_CREATE, set the managed
value to YES if not already set to indicate to the deleteVport code that
it should delete the vHBA when the pool is destroyed.
If libvirtd is restarted all the memory only state was lost, so for a
persistent storage pool, use the virStoragePoolSaveConfig in order to
write out the managed value.
Because we're now saving the current configuration, we need to be sure
to not save the parent in the output XML if it was undefined at start.
Saving the name would cause future starts to always use the same parent
which is not the expected result when not providing a parent. By not
providing a parent, libvirt is expected to find the best available
vHBA port for each subsequent (re)start.
At deleteVport, use the new managed value to decide whether to execute
the VPORT_DELETE. Since we no longer save the parent in memory or in
XML when provided, if it was not provided, then we have to look it up.
https://bugzilla.redhat.com/show_bug.cgi?id=1160926
Passing a copy of the storage pool adapter to a function just changes the
copy of the fields in the particular function and then when returning to
the caller those changes are discarded. While not yet biting us in the
storage clean-up case, it did cause an issue for the fchost storage pool
startup case, createVport. The issue was at startup, if no parent is found
in the XML, the code will search for the 'best available' parent and then
store that in the in memory copy of the adapter. Of course, in this case
it was a copy, so when returning to the virStorageBackendSCSIStartPool that
change was discarded (or lost) from the pool->def->source.adapter which
meant at shutdown (deleteVport), the code assumed no adapter was passed
and skipped the deletion, leaving the vHBA created by libvirt still defined
requiring an additional stop of a nodedev-destroy to remove.
Adjusted the createVport to take virStoragePoolDefPtr instead of the
adapter copy. Then use the virStoragePoolSourceAdapterPtr when processing.
A future patch will need the 'def' anyway, so this just sets up for that.
https://bugzilla.redhat.com/show_bug.cgi?id=1160565
The existing code assumed that the configuration of a 'parent' attribute
was correct for the createVport path. As it turns out, that may not be
the case which leads errors during the deleteVport path because the
wwnn/wwpn isn't associated with the parent.
With this change the following is reported:
error: Failed to start pool fc_pool_host3
error: XML error: Parent attribute 'scsi_host4' does not match parent 'scsi_host3' determined for the 'scsi_host16' wwnn/wwpn lookup.
for XML as follows:
<pool type='scsi'>
<name>fc_pool</name>
<source>
<adapter type='fc_host' parent='scsi_host4' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>
</source>
Where 'nodedev-dumpxml scsi_host16' provides:
<device>
<name>scsi_host16</name>
<path>/sys/devices/pci0000:00/0000:00:04.0/0000:10:00.0/host3/vport-3:0-11/host16</path>
<parent>scsi_host3</parent>
<capability type='scsi_host'>
<host>16</host>
<unique_id>13</unique_id>
<capability type='fc_host'>
<wwnn>5001a4aaf3ca174b</wwnn>
<wwpn>5001a4a77192b864</wwpn>
...
The patch also adjusts the description of the storage pool to describe the
restrictions.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160565
If a 'parent' attribute is provided for the fchost, then at startup
time check to ensure it is a vport capable scsi_host. If the parent
is not vport capable, then disallow the startup. The following is the
expected results:
error: Failed to start pool fc_pool
error: XML error: parent 'scsi_host2' specified for vHBA is not vport capable
where the XML for the fc_pool is:
<pool type='scsi'>
<name>fc_pool</name>
<source>
<adapter type='fc_host' parent='scsi_host2' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>
</source>
...
and 'scsi_host2' is not vport capable.
Providing an incorrect parent and a correct wwnn/wwpn could lead to
failures at shutdown (deleteVport) where the assumption is the parent
is for the fchost.
NOTE: If the provided wwnn/wwpn doesn't resolve to an existing scsi_host,
then we will be creating one with code (virManageVport) which
assumes the parent is vport capable.
Signed-off-by: John Ferlan <jferlan@redhat.com>
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.
Use the device type name if we know it instead of its number,
even if we can't hotplug it:
qemuMonitorJSONAttachCharDevCommand:6094 : operation failed: Unsupported
char device type '10'
virDomainChrSourceDefIsEqual should return 'true' for
identical SPICEVMC chardevs, and those that have no source
specification.
After this change, a failed hotplug no longer leaves a stale
pointer in the domain definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1162097
If the memory mode is specified as 'strict' and with one node, we
get the following error when starting domain.
error: Unable to write to '$cgroup_path/cpuset.mems': Device or resource busy
XML is configured with numatune as follows:
<numatune>
<memory mode='strict' nodeset='0'/>
</numatune>
It's broken by Commit 411cea638f
which moved qemuSetupCgroupForEmulator() before setting cpuset.mems
in qemuSetupCgroupPostInit.
Directory '$cgroup_path/emulator/' is created in qemuSetupCgroupForEmulator.
But '$cgroup_path/emulator/cpuset.mems' it not set and has a default value
(all nodes, such as 0-1). Then we setup '$cgroup_path/cpuset.mems' to the
nodemask (in this case it's '0') in qemuSetupCgroupPostInit. It must fail.
This patch makes '$cgroup_path/emulator/cpuset.mems' is set before
'$cgroup_path/cpuset.mems'. The action is similar with that in
qemuDomainSetNumaParamsLive.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
If the memory mode in numatune is not 'strict', we should not setup
cpuset.mems. Before commit 1a7be8c600
we have checked the memory mode in virDomainNumatuneGetNodeset. This
patch adds the check as before.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
If the memory mode in numatune is specified as 'preferred' with one node
(such as nodeset='0'), domain's memory is not all in node 0 absolutely.
Assumption that node 0 doesn't have enough memory, memory can be allocated
on node 1 when qemu process startup. Then if we set cpuset.mems to '0',
it may invoke OOM.
Commit 1a7be8c600 changed the former logic of
checking memory mode in virDomainNumatuneGetNodeset. This patch adds the
check as before.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
This patch fixes the following issues.
1) When an invalid wwn is introduced, libvirt reports
"Malformed wwn: %s". The template won't be replaced.
2) "target" option for dompmsuspend and "xml" option for
save-image-define are required options and should use
VSH_OT_DATA instead of VSH_OT_STRING as an option type.
3) A typo.
Signed-off-by: Hao Liu <hliu@redhat.com>
