1) virNetworkObjUpdate should be an all or none operation, but in the
case that we want to update both the live state and persistent config
versions of the network, it was committing the update to the live
state before starting to update the persistent config. If update of
the persistent config failed, we would leave with things in an
inconsistent state - the live state would be updated (even though an
error was returned), but persistent config unchanged.
This patch changed virNetworkObjUpdate to use a separate pointer for
each copy of the virNetworkDef, and not commit either of them in the
virNetworkObj until both live and config parts of the update have
successfully completed.
2) The parsers for various pieces of the virNetworkDef have all sorts
of subtle limitations on them that may not be known by the
Update[section] function, making it possible for one of these
functions to make a modification directly to the object that may not
pass the scrutiny of a subsequent parse. But normally another parse
wouldn't be done on the data until the *next* time the object was
updated (which could leave the network definition in an unusable
state).
Rather than fighting the losing battle of trying to duplicate all the
checks from the parsers into the update functions as well, the more
foolproof solution to this is to simply do an extra
virNetworkDefCopy() operation on the updated networkdef -
virNetworkDefCopy() does a virNetworkFormat() followed by a
virNetworkParseString(), so it will do all the checks we need. If this
fails, then we don't commit the changed def.
The bridge driver implementation of virNetworkUpdate() removes and
re-adds iptables rules any time a network has an <ip>, <forward>, or
<forward>/<interface> element updated. There are some types of
networks that have those elements and yet have no iptables rules
associated with them, and unfortunately the functions that remove/add
iptables rules don't check the type of network before attempting to
remove/add the rules, sometimes leading to an erroneous failure of the
entire update operation.
Under normal circumstances I would refactor the lower level functions
to be more robust, but to avoid code churn as much as possible, I've
just added extra checks directly to networkUpdate().
Nothing uses the return value, and creating it requries otherwise
unnecessary strlen () calls.
This cleanup is conceptually independent from the rest of the series
(although the later patches won't apply without it). This just seems
a good opportunity to clean this up, instead of entrenching the unnecessary
return value in the virLogOutputFunc instance that will be added in this
series.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
maxcpu and hostcpus are defined and calculated in qemudDomainPinVcpuFlags()
and qemudDomainPinEmulator(), but never used. So remove them including nodeinfo.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
This allows the user to control labelling of each character device
separately (the default is to inherit from the VM).
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
This is just code motion, allowing us to reuse the same function to
parse the <seclabel> from character devices too.
However it also fixes a possible segfault in the original code if
VIR_ALLOC_N returns an error and the cleanup code (at the error:
label) tries to iterate over the unallocated array (thanks Michal
Privoznik for spotting this).
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Disk hotplug is a two phase action: qemuMonitorAddDrive followed by
qemuMonitorAddDevice. When the first part succeeds but the second one
fails, we need to rollback the drive addition.
The README file seems to be a leftover from some previous version of
locking driver. It is not consistent with what the code does nor is it
consistent with existing documentation in internals/locking.html.
Some kernel versions (at least RHEL-6 2.6.32) do not let you over-mount
an existing selinuxfs instance with a new one. Thus we must unmount the
existing instance inside our namespace.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When auto-probing hypervisor drivers, the conn->uri field will
initially be NULL. Care must be taken not to access members
when doing auth lookups in the config file
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
portgroup elements are located in the toplevel of <network>
objects. There can be multiple <portgroup> elements, and they each
have a unique name attribute.
Add, delete, and modify are all supported for portgroup. When deleting
a portgroup, only the name must be specified in the provided xml - all
other attributes and subelements are ignored for the purposes of
matching and existing portgroup.
The bridge driver and virsh already know about the portgroup element,
so providing this backend should cause the entire stack to work. Note
that in the case of portgroup, there is no external daemon based on
the portgroup config, so nothing must be restarted.
It is important to note that guests make a copy of the appropriate
network's portgroup data when they are started, so although an updated
portgroup's configuration will have an affect on new guests started
after the cahange, existing guests won't magically have their
bandwidth changed, for example. If something like that is desired, it
will take a lot of redesign work in the way network devices are setup
(there is currently no link from the network back to the individual
interfaces using it, much less from a portgroup within a network back
to the individual interfaces).
The dhcp range element is contained in the <dhcp> element of one of a
network's <ip> elements. There can be multiple <range>
elements. Because there are only two attributes (start and end), and
those are exactly what you would use to identify a particular range,
it doesn't really make sense to modify an existing element, so
VIR_NETWORK_UPDATE_COMMAND_MODIFY isn't supported for this section,
only ADD_FIRST, ADD_LAST, and DELETE.
Since virsh already has support for understanding all the defined
sections, this new backend is automatically supported by virsh. You
would use it like this:
virsh net-update mynet add ip-dhcp-range \
"<range start='1.2.3.4' end='1.2.3.20'/>" --live --config
The bridge driver also already supports all sections, so it's doing
the correct thing in this case as well - since the dhcp range is
placed on the dnsmasq commandline, the bridge driver recreates the
dnsmasq commandline, and re-runs dnsmasq whenever a range is
added/deleted (and AFFECT_LIVE is specified in the flags).
This command uses the new virNetworkUpdate() API to modify an existing
network definition, and optionally have those modifications take
effect immediately without restarting the network.
An example usage:
virsh net-update mynet add-last ip-dhcp-host \
"<host mac='00:11:22:33:44:55' ip='192.168.122.45'/>" \
--live --config
If you like, you can instead put the xml into a file, and call like
this:
virsh net-update mynet add ip-dhcp-host /tmp/myxml.xml
--live --config
virsh will autodetect whether the argument is itself an xml element,
or if it's a file, by looking at the first character - the first
character of an xml element is always "<", and the first character of
a file is almost always *not* "<" (in the rare case that it is, the
user could specify "./<filename...").
A --parent-index option is also available (to give the index within a
list of parent objects, e.g. the index of the parent <ip> element when
updating ip-dhcp-host elements), but is optional and at least for now
will probably be used rarely.
--live, --config, and --current options - if you specify --live, only
the live state of the network will be updated. If you also specify
--config, then the persistent configuration will also be updated;
these two commands can be given separately, or both together. If you
don't specify either (you can optionally specify "--current" for the
same effect), then the "current" config will be updated (i.e. if the
network is active, then only its live config is affected, but if the
network is inactive, only the persistent config is affected).
Noticed this by reading the page. It would be so much nicer if our
tools could automatically flag things like this as part of 'make'.
* docs/formatdomain.html.in: Remove extra '>'.
https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.
* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/; If/. If/
This patch adds to relatedlinks.html a link to an article about libvirt
describing how the Linux audit subsystem can be used to track qemu
guest's life-cycle.
If any of the bootstrap tasks (autoconf/automake/etc) failed,
autogen.sh carried on running any pre-existing configure anyway.
Use 'set -e' to ensure autogen.sh immediately exists on error.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The "dump-guest-core' option is new option for the machine type
(-machine pc,dump-guest-core) that controls whether the guest memory
will be marked as dumpable.
While testing this, I've found out that the value for the '-M' options
is not parsed correctly when additional parameters are used. However,
when '-machine' is used for the same options, it gets parsed as
expected. That's why this patch also modifies the parsing and creating
of the command line, so both '-M' and '-machine' are recognized. In
QEMU's help there is only mention of the 'machine parameter now with
no sign of the older '-M'.
Sometimes when guest machine crashes, coredump can get huge due to the
guest memory. This can be limited using madvise(2) system call and is
being used in QEMU hypervisor. This patch adds an option for configuring
that in the domain XML and related documentation.
Whenever the guest machine fails to boot, new parameter (reboot-timeout)
controls whether it should reboot and after how many ms it should do so.
Docs included.
The DAC security driver silently ignored errors when parsing the DAC
label and used default values instead.
With a domain containing the following label definition:
<seclabel type='static' model='dac' relabel='yes'>
<label>sdfklsdjlfjklsdjkl</label>
</seclabel>
the domain would start normaly but the disk images would be still owned
by root and no error was displayed.
This patch changes the behavior if the parsing of the label fails (note
that a not present label is not a failure and in this case the default
label should be used) the error isn't masked but is raised that causes
the domain start to fail with a descriptive error message:
virsh # start tr
error: Failed to start domain tr
error: internal error invalid argument: failed to parse DAC seclabel
'sdfklsdjlfjklsdjkl' for domain 'tr'
I also changed the error code to "invalid argument" from "internal
error" and tweaked the various error messages to contain correct and
useful information.
This patch cleans up building the "-boot" parameter and while on that
fixes one inconsistency by modifying these things:
- I completed the unfinished virDomainBootMenu enum by specifying
LAST, declaring it and also declaring the TypeFromString and
TypeToString parameters.
- Previously mentioned TypeFromString and TypeToString are used when
parsing the XML.
- Last, but not least, visible change is that the "-boot" parameter
is built and parsed properly:
- The "order=" prefix is used only when additional parameters are
used (menu, etc.).
- It's rewritten in a way that other parameters can be added
easily in the future (used in following patch).
- The "order=" parameter is properly parsed regardless to where it
is placed in the string (e.g. "menu=on,order=nc").
- The "menu=" parameter (and others in the future) are created
when they should be (i.e. even when bootindex is supported and
used, but not when bootloader is selected).
Currently, we mark domain PAUSED (but not emit an event)
just before we issue 'stop' on monitor; This command can
take ages to finish, esp. when domain's doing a lot of
IO - users can enforce qemu to open files with O_DIRECT
which doesn't return from write() until data reaches the
block device. Having said that, we report PAUSED even if
domain is not paused yet.
The memmove to move elements in the dhcp hosts array when inserting
and deleting items was mistakenly basing the length of the copy on the
size of a virNetworkDHCPHostDefPtr rather than virNetworkDHCPHostDef,
with the expected disastrous results.
The memmove to delete an entry commits two errors - along with the
size of each element being wrong, it also omits some required
parentheses.
These enums originally were put into the flags for virNetworkUpdate,
and when they were moved into their own enum, the numbers weren't
appropriately changed, causing the commands to start with value 2
instead of 1. This causes problems for things like ENUM_IMPL, which
wants a string for every value in the requested range, including those
not used in the enum.
Based exclusively on work by Eric Blake in a patch posted with the same
subject. However some modifications related to comments and my plans to
add another backend.
Added WITH_INTERFACE as the only automake variable deciding whether to
build the driver and using WITH_NETCF to identify that we're wanting to
use the netcf library as the backend.
* configure.ac: Added with_interface
* src/interface/netcf_driver.c: Renamed..
* src/interface/interface_backend_netcf.c: ..to this to match storage.
* src/interface/netcf_driver.h: Renamed..
* src/interface/interface_driver.h: ..to this.
* daemon/Makefile.am: Respect WITH_INTERFACE and WITH_NETCF.
* libvirt.spec.in: Add RPM support for --with-interface
Commit f36309d added an export with no matching implementation;
probably a misspelling of an earlier version of the final addition
of virNetworkObjSetDefTransient.
* src/libvirt_private.syms (network_conf.h): Drop bogus
virNetworkSetDefTransient.
Commit aaa8ab3 added new static functions that are only used on Linux;
but commit 22acfdc didn't go far enough to fix compiler issues.
* src/nodeinfo.c (nodeSetMemoryParameterValue)
(nodeGetMemoryParameterValue): Conditionally compile based on use.
Commit ee3d3893 missed the fact that (unsigned char)<<(int)
is truncated to int, and therefore failed for any bitmap data
longer than four bytes.
Also, I failed to run 'make syntax-check' on my commit 4bba6579;
for whatever odd reason, ffs lives in a different header than ffsl.
* src/util/bitmap.c (virBitmapNewData): Use correct shift type.
(includes): Glibc (and therefore gnulib) decided ffs is in
<strings.h>, but ffsl is in <string.h>.
* tests/virbitmaptest.c (test5): Test it.
Commit 0fc89098 used functions only available on glibc, completely
botched 32-bit environments, and risked SIGBUS due to unaligned
memory access on platforms that aren't as forgiving as x86_64.
* bootstrap.conf (gnulib_modules): Import ffsl.
* src/util/bitmap.c (includes): Use <strings.h> for ffsl.
(virBitmapNewData, virBitmapToData): Avoid 64-bit assumptions and
non-portable functions.
The introduction of APIC EOI patches had a few little details that
could look better, so this patch fixes that and one more place in the
file as well (same problem).
When trying to get the value of a private secret, the code used
'operation denied' error. That error is specified as a error for
read-only connections trying to perform denied operation. The
following error seems more accurate.
To compare the difference:
- BEFORE
error: operation secret is private forbidden for read only access
- AFTER
error: Invalid secret: secret is private
Two changes are introduced in this patch:
- The first change removes ATTRIBUTE_RETURN_CHECK from
virNetDevBandwidthClear, because it was called with ignore_value
always, anyway. The function is used even when it's not necessary
to call it, just for cleanup purposes.
- The second change is added ignoring of the command's exit status,
since it may report an error even when run just as "to be sure we
clean up" function. No libvirt errors are suppresed by this.
With this script you can run libvirt programs without needing to
install them first. You just have to do for example:
./run ./tools/virsh [args ...]
If you are already in the tools/ subdirectory, then the following
command will also work:
../run ./virsh [...]
You can also run the C programs under valgrind like this:
./run valgrind [valgrind opts...] ./program
or under gdb:
./run gdb --args ./program
This also works with sudo (eg. if you need root access for libvirt):
sudo ./run ./tools/virsh list --all
Derived from libguestfs and simplified. The ./run script in
libguestfs is much more sophisticated:
https://github.com/libguestfs/libguestfs/blob/master/run.in
Commit 7a99b0abaf adds a new RPC struct
but one of the members has different names in remote_protocol.x and
remote_protocol-struct breaking make check.
