When calling qemuProcessKill from the virDomainDestroy impl
in QEMU, do not ignore the return value. This ensures that
if QEMU fails to respond to SIGKILL, the caller will know
about the failure.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Depending on the scenario in which LXC containers exit, it is
possible for the EOF callback of the LXC monitor to deadlock
the driver.
#0 0x00000038a0a0de4d in __lll_lock_wait () from /lib64/libpthread.so.0
#1 0x00000038a0a09ca6 in _L_lock_840 () from /lib64/libpthread.so.0
#2 0x00000038a0a09ba8 in pthread_mutex_lock () from /lib64/libpthread.so.0
#3 0x00007f4bd9579d55 in virMutexLock (m=<optimized out>) at util/threads-pthread.c:85
#4 0x00007f4bcacc7597 in lxcDriverLock (driver=0x7f4bc40c8290) at lxc/lxc_conf.h:81
#5 virLXCProcessMonitorEOFNotify (mon=<optimized out>, vm=0x7f4bb4000b00) at lxc/lxc_process.c:581
#6 0x00007f4bd9645c91 in virNetClientCloseLocked (client=client@entry=0x7f4bb4009e60)
at rpc/virnetclient.c:554
#7 0x00007f4bd96460f8 in virNetClientIOEventLoopPassTheBuck (thiscall=0x0, client=0x7f4bb4009e60)
at rpc/virnetclient.c:1306
#8 virNetClientIOEventLoopPassTheBuck (client=0x7f4bb4009e60, thiscall=0x0)
at rpc/virnetclient.c:1287
#9 0x00007f4bd96467a2 in virNetClientCloseInternal (reason=3, client=0x7f4bb4009e60)
at rpc/virnetclient.c:589
#10 virNetClientCloseInternal (client=0x7f4bb4009e60, reason=3) at rpc/virnetclient.c:561
#11 0x00007f4bcacc4a82 in virLXCMonitorClose (mon=0x7f4bb4000a00) at lxc/lxc_monitor.c:201
#12 0x00007f4bcacc55ac in virLXCProcessCleanup (reason=<optimized out>, vm=0x7f4bb4000b00,
driver=0x7f4bc40c8290) at lxc/lxc_process.c:240
#13 virLXCProcessStop (driver=0x7f4bc40c8290, vm=vm@entry=0x7f4bb4000b00,
reason=reason@entry=VIR_DOMAIN_SHUTOFF_DESTROYED) at lxc/lxc_process.c:735
#14 0x00007f4bcacc5bd2 in virLXCProcessAutoDestroyDom (payload=<optimized out>,
name=0x7f4bb4003c80, opaque=0x7fff41af2df0) at lxc/lxc_process.c:94
#15 0x00007f4bd9586649 in virHashForEach (table=0x7f4bc409b270,
iter=iter@entry=0x7f4bcacc5ab0 <virLXCProcessAutoDestroyDom>, data=data@entry=0x7fff41af2df0)
at util/virhash.c:514
#16 0x00007f4bcacc52d7 in virLXCProcessAutoDestroyRun (driver=driver@entry=0x7f4bc40c8290,
conn=conn@entry=0x7f4bb8000ab0) at lxc/lxc_process.c:120
#17 0x00007f4bcacca628 in lxcClose (conn=0x7f4bb8000ab0) at lxc/lxc_driver.c:128
#18 0x00007f4bd95e67ab in virReleaseConnect (conn=conn@entry=0x7f4bb8000ab0) at datatypes.c:114
When the driver calls virLXCMonitorClose, there is really no
need for the EOF callback to be invoked in this case, since
the caller can easily handle events itself. In changing this,
the monitor needs to take a deep copy of the callback list,
not merely a reference.
Also adds debug statements in various places to aid
troubleshooting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
<interface> elements are location inside the <forward> element of a
network. There is only one <forward> element in any network, but it
might have many <interface> elements. This element only contains a
single attribute, "dev", which is the name of a network device
(e.g. "eth0").
Since there is only a single attribute, the modify operation isn't
supported for this "section", only add-first, add-last, and
delete. Also, note that it's not permitted to delete an interface from
the list while any guest is using it. We may later decide this is safe
(because removing it from the list really only excludes it from
consideration in future guest allocations of interfaces, but doesn't
affect any guests currently connected), but for now this limitation
seems prudent (of course when changing the persistent config, this
limitation doesn't apply, because the persistent config doesn't
support the concept of "in used").
Another limitation - it is also possible for the interfraces in this
list to be described by PCI address rather than netdev name. However,
I noticed while writing this function that we currently don't support
defining interfaces that way in config - the only method of getting
interfaces specified as <adress type='pci' ..../> instead of
<interface dev='xx'/> is to provide a <pf dev='yy'/> element under
forward, and let the entries in the interface list be automatically
populated with the virtual functions (VF) of the physical function
device given in <pg>.
As with the other virNetworkUpdate section backends, support for this
section is completely contained within a single static function, no
other changes were required, and only functions already called from
elsewhere within the same file are used in the new content for this
existing function (i.e., adding this code should not cause a new build
problem on any platform).
Jim Fehlig reported a compilation error with older gcc 4.3.4:
libvirt.c: In function 'virDomainGetEmulatorPinInfo':
libvirt.c:9111: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
It looks like someone programmed via too much copy-and-paste.
* src/libvirt.c (virDomainGetEmulatorPinInfo): Multiplying by 1 is
a no-op, and thus will never overflow.
Recently, there have been some improvements made to qemu so it
supports seamless migration or something very close to it.
However, it requires libvirt interaction. Once qemu is migrated,
the SPICE server needs to send its internal state to the destination.
Once it's done, it fires SPICE_MIGRATE_COMPLETED event and this
fact is advertised in 'query-spice' output as well.
We must not kill qemu until SPICE server finishes the transfer.
SELinux wants all log files opened with O_APPEND. When
running non-root though, libvirtd likes to use O_TRUNC
to avoid log files growing in size indefinitely. Instead
of using O_TRUNC though, we can use O_APPEND and then
call ftruncate() which keeps SELinux happier.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Asynchronously setting priv->mon to NULL was pointless,
just remove the destroy callback entirely.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Continue consolidation of process functions by moving some
helpers out of command.{c,h} into virprocess.{c,h}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are a number of process related functions spread
across multiple files. Start to consolidate them by
creating a virprocess.{c,h} file
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virCommand prefix was inappropriate because the API
does not use any virCommandPtr object instance. This
API closely related to waitpid/exit, so use virProcess
as the prefix
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In most of the snapshot API's there's no need to hold the driver lock
the whole time.
This patch adds helper functions that get the domain object in functions
that don't require the driver lock and simplifies call paths from
snapshot-related API's.
It might need some time till the LUN's stable path shows up on
initiator host, and although the time window is not foreseeable,
as a better than nothing fix, this patch adds timeout for the
stable path discovery process.
Commit 9298bfbcb introduced code to detect if netcf is linked with
libnl1, and to prefer libnl1 over libnl3 when this is the case.
This behaviour can be disabled by setting LIBNL_CFLAGS to any value,
including the empty string.
However, configure.ac sets LIBNL_CFLAGS to "" before attempting
libnl detection, so the libnl1 detection code is always disabled.
This caused issues on my f17 system where netcf is linked with libnl1
but libvirt got built with libnl3.
This commit removes the setting of the LIBNL_* variables to "" as
this does not appear to be needed. After this change, libnl1 is
used when building libvirt on my f17 system.
The Fedora policies don't want us installing the legacy initscripts
in parallel with the systemd ones, so switch to only install the
systemd unit
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Every level of the code for virNetworkUpdate was assuming that some
other level was checking for validity of the "command" arg, but none
actually were. The result was that an invalid command code would do
nothing, but also report success.
Since the command code isn't used until the very lowest level backend
functions, that's where I put the check. I made a separate one-line
function to log the error. The compiler would have combined the
identical strings used by multiple calls if I'd just called
virReportError directly in each location, but sending them all to the
same string in the source guards against inadvertant divergence (which
would lead to extra work for translators.)
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 '>'.
